26.1 Snapshots

[paper cedar]

Why would you pass in TypedInstance?

[plucky hollow]

first one that came to mind

[paper cedar]

I'll try my best to update the time thing

but i have no idea what the original PR actually wanted to add

[wintry flower]

I guess why would we have put it elsewhere

validateComponents checks that different components are coherent, whereas our check can/could be implemented locally

[paper cedar]

ya and the stack traces are more usable

re time stuff: Found the original PR at least (https://github.com/neoforged/NeoForge/pull/1318)

[wintry flower]

We could add a function to ItemResource that returns the inner stack as an ItemInstance

But if we do that we should mirror the whole ItemInstance thing to fluids

[paper cedar]

Oh yes, interesting thought. And yes, we should.

[wintry flower]

It is not quite an ItemInstance since it doesn't have a count, but some simple asItemInstance()-like function would bridge the gap nicely

[graceful laurel]

Isn't this wrong? Shouldn't this use the smaller of the dimensions instead of the larger to compute the max mip level?

[gray frigate]

no, that matches the math done in Vulkan's validation layers
https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/7cf5219493167736068d2be01774fc920d3f4d1c/layers/stateless/sl_image.cpp#L137L146

max_dim = max(w, h, d);
max_mip_levels = static_cast<uint32>((floor(log2(max_dim)) + 1);

when one dimension hits 1, the other dimensions can still be compressed into another mip level

its only once you get down to 1x1x1 that you can't create another mip level

[graceful laurel]

Fair enough

[paper cedar]

Hm. Might be a vanilla bug since this system is very new

A dimension can have "no clock", but this.awardStat(Stats.TIME_SINCE_REST); will still be awarded hm

[paper cedar]

Whew I am trying my best to restore this system but it'll need to be reviewed hehe

Huh. Interesting new feature on registries. A query by component: return BuiltInRegistries.ITEM.componentLookup().findMatching(DataComponents.ENTITY_DATA, c -> c.type() == type).findAny();

[runic anchor]

is the -3 snapshot's datagen runnable rn

(in neoforge)

[paper cedar]

Hmmm, it should be

Oh wait -3, no nvm.

-3 isn't yet

[runic anchor]

I have this change in local 26.1-3 neo but just need neo buildable so I can run datagen before I can push commit
https://github.com/FabricMC/fabric-api/pull/5135

[paper cedar]

Yeah 26.1-3 is not baked yet

[plucky hollow]

left some comments on the 26.1-3 port pr, some of the suggestions xfact gave were not correct
they were suggesting to use FarmlandBlock (renamed from FarmBlock) but this snapshot introduced a bunch of new crop block tags which should be used instead, used snowman for sources

[paper cedar]

Yeah, see above, and yes, did the same 😄

Also looked at snowman for the delta

I don't know about canHydrate (which is used for sugarcane)

I left our hook in place in addition to the new vanilla tag check

[runic anchor]

you got everything I was going to say apex. All instanceof farmland should be yeeted to whatever vanilla has now

we can always repatch later but it looks like vanilla covered everything

[paper cedar]

Yeah, agreed that seems sensible. The tag check is more powerful than the instanceof

In the sense that the class-hierarchy is static while tags can be modified

Alright. It compiled again, I pushed my work so far.

I definitely made some mistakes when reinstating the variable daytime patches, since the vanilla system now works very differently (multiple separate clocks, the level picks wich clock it uses, etc.)

Heh

Alright, so every testframework class that does something like this will now crash on startup:

    private static final ItemStack MOCK_OUTPUT = new ItemStack(Items.COBBLESTONE);

With:

Caused by: java.lang.ExceptionInInitializerError: Exception java.lang.NullPointerException: Components not bound yet [in thread "modloading-worker-0"]
    at java.base/java.util.Objects.requireNonNull(Objects.java:246)
    at TRANSFORMER/[email protected]/net.minecraft.core.Holder$Reference.components(Holder.java:273)
    at TRANSFORMER/[email protected]/net.minecraft.world.item.ItemStack.<init>(ItemStack.java:255)
    at TRANSFORMER/[email protected]/net.minecraft.world.item.ItemStack.<init>(ItemStack.java:251)
    at TRANSFORMER/neotests@1/net.neoforged.neoforge.debug.crafting.AnvilUpdateEventTests.<clinit>(AnvilUpdateEventTests.java:40)

[plucky hollow]

deferred itemstacks construction when?

[paper cedar]

Hehehehe, the cases I've found so far didn't really need to be static anyway

Oh, also, come to think of it. You could create an ItemStackTemplate, presumably

And then "realize" it with create()

Yup, also crashes hard: private static final ItemResource RESOURCE = ItemResource.of(Items.APPLE);

Okay, datagen works sometimes now.

Aaaaaah nevermind, no, the testframework datagen crashes, but the main one works.

[plucky hollow]

why did you make DeferredHolder.components throw on missing registry?
shouldnt that work just fine if you pass false, missing registry would result in null holder, returning the empty data map

[paper cedar]

I wasn't really sure if you want it to do that instead of throw

Since you can call areComponentsBound() first to check, no?

But: low confidence, that may be incorrect

[plucky hollow]

the only place i personally would expect to throw if unbound would be #value but that might just be me

[paper cedar]

It will throw in holder.components() anyway

Namely the exception I pasted above

[plucky hollow]

ah then sure if thats what vanilla does

[paper cedar]

But granted, it will throw for a different reason

[plucky hollow]

people rly shouldnt be calling components that early anyways so doesnt rly matter

[paper cedar]

Hm, so. Interesting.
Our TestsMod calls framework.init() in its mod constructor

that is before registries are setup, meaning it will crash if it tries to access any ItemStack in any of the gametest setup methods

[plucky hollow]

does it need to be that early?

[paper cedar]

I think it's for event tests, so probably yes

Uuuh. Yeah. There are a few problems now...

This for example doesn't work, since during Item registration the components can't be bound yet, obviously:

BACKPACK = ITEMS.registerItem("test_backpack", Item::new, props -> {
    NonNullList<ItemStack> defaultContents = NonNullList.withSize(SLOTS, ItemStack.EMPTY);
    defaultContents.set(STICK_SLOT, Items.STICK.getDefaultInstance().copyWithCount(64)); <-- access to getDefaultInstance() crashes
    return props.component(DataComponents.CONTAINER, ItemContainerContents.fromItems(defaultContents));
});

[plucky hollow]

shouldnt item properties be using delayedComponent now not component?

delayedComponent(DataComponentType<T> type, DataComponentInitializers.SingleComponentInitializer<T> initializer)
would maybe be useful here, return ItemContainerContents.fromItems within the initializer

[paper cedar]

yep good point

[plucky hollow]

maybe worth deprecating the component method poitning people to delayedComponent?

[empty night]

I am SO late, hi

This is fun

[plucky hollow]

oh yeah nano your so late

[rain herald]

apparently very broken

[plucky hollow]

btw @empty night holders have components now

have fun looking at that

[empty night]

Bruh. I JUST started updating my transfer stuff

[paper cedar]

Well technically the default components for the held value

[empty night]

Now we gotta rip up components again?

[plucky hollow]

yeah its where items default components moved to
and is moving more towards any registry having components

[paper cedar]

It's mostly around how the default components for objects are done IMHO

[empty night]

Also. World Options. Mojang if you're gonna add data packs to that and make resync happen...

It'd finally help unfuck some of creative menu

[plucky hollow]

oh no you would still grab components from your stack
but Item does not track the default values anymore

so not much ripping out from that perspective

[empty night]

Oh good

[plucky hollow]

but this is moving towards cursed things like menuTypeHolder.getComponent being possible

[plucky hollow]

which menus having components now that i think of it could be useful
imagine the lock code system being a component on your menu

[empty night]

26.1 is gonna destroy everyone that isn't porting lol

[plucky hollow]

oh yeah

[empty night]

CC4 is gonna suuuuuuck

[plucky hollow]

if your not porting people, please start sooner rather thabn later

[paper cedar]

Okay, what
Components aren't bound when datagen runs, that can't be, how would the neo datagen run in that case argh

[plucky hollow]

yeah that doesnt sound right

[paper cedar]

But it is not the menu that has components, it's the type

which is less useful

So, uh, conceptual problem here

The default components are bound when server resources reload

I don't think we do that for datagen

For kinda obvious recursive reasons 😄

[plucky hollow]

wouldnt be the first time datagen had to init something itself

[paper cedar]

Hm, Vanilla seems to not use a lot of Itemstack in their datagen anymore

[plucky hollow]

culd we bind the components in DatagenModloader
before RegistrationEvents.collectComponentModifiers();?

unsure how the binding works

[paper cedar]

They moved to ItemStackTemplate

[plucky hollow]

makes sense tbh no need for datagend stacks to be mutable

[paper cedar]

They also never access the bound components unless you try to read from it

Heh, so I did try to bind the components, but this is still recursive:

BACKPACK = ITEMS.registerItem("test_backpack", Item::new, props -> {
    return props.delayedComponent(DataComponents.CONTAINER, context -> {
        NonNullList<ItemStack> defaultContents = NonNullList.withSize(SLOTS, ItemStack.EMPTY);
        defaultContents.set(STICK_SLOT, Items.STICK.getDefaultInstance().copyWithCount(64));                
        return ItemContainerContents.fromItems(defaultContents);
    });
});

Since Items.STICK.getDefaultInstance() requires components to be bound, but delayedComponent is invoked when components are being bound

[plucky hollow]

does itemcontainer contents support the new template?

[paper cedar]

Yes, but no public factory-method or constructor is exposed for it, weirdly enough

Internally it does store List<Optional<ItemStackTemplate>> items;

[plucky hollow]

might need to patch one in if using itemstack is the pain point?

[paper cedar]

Yeah potentially, but I am not sure yet

[plucky hollow]

i can potentially see alot of these kinds of patches, where we need to use template instead of stack

[plucky hollow]

also, does the default value need to have a stack in that slot?
can you not set stick into the slot before doing anything in the tests?

from what im seeing the item can be registered with empty contents
and then right after BACKPACK.toStack
you can set the stick into STICK_SLOT

[paper cedar]

Yeah probably, I was just grinding through the startup exceptions

I need to revisit the clock manager once the game actually runs... I think I've added extended-state in several places but might not actually be using that when the clock manager is saved

[graceful laurel]

Those suggestions were specifically intended as a "make it compile" solution as it seemed there was still some discussion about how to proceed with these patches and the new tags

[paper cedar]

Hm, ItemStack/ItemStackTemplate pose an interesting question for datagen for sure. I.e. we add some ItemStack based extensions to the providers, do we remove those?

[lucid tree]

What exactly does the Clock stuff do?

[indigo venture]

keeps track of time

[lucid tree]

Hmm

Also did DataComponents get changed?

[indigo venture]

each clock is separate and can be paused/resumed/set differently

and can be used for things like loot table predicates

or some built in ones for villager hours and daylight cycle

[lucid tree]

Neat

[paper cedar]

I think our patches to allow variable-speed daytime should just be clock-agnostic (and be configurable separately for each clock)

[north berry]

https://www.youtube.com/watch?v=-KG_AMMz4PQ

[hazy wind]

why the fuck did this give me a French virgin ad?

[wintry flower]

We can remove them if templates can be provided

If they're limited to item and count then we need to continue providing a way to generate a result with components

[paper cedar]

Internally the RecipeProvider for example seems to use an ItemStackTemplate already

ItemStack might be more convenient if your Mod API already has ways to make stacks

If we find a reliable way to bind the default components that'll be fine

[plucky hollow]

just to note ItemStackTemplate is record(Holder<Item> item, int count, DataComponentPatch components)
so they can be used to generate stacks with components just fine i would think

they are essentially our ItemResource but with a count property from what i see/understand

[paper cedar]

Yes but he was refering to the provider internally still using ItemLike+count I think

[plucky hollow]

oh i thought was asking if the template supported components, or thats at least how i read it

but imo the move to using template/item instance in more places is a good one

also i just noticed ItemInstance is not ItemLike, huh i would think it would be

[paper cedar]

That might lead to ambiguity in some places

[plucky hollow]

yeah thats true, looking at stuff you dont wanna do new ItemStack(itemStack) (ItemLike ctor)

[empty night]

I'd take ItemStack(ItemStack) - that's just a clone function at that point

[alpine minnow]

Did a new snapshot drop?

Snapshot 3?

[zenith dove]

yes

[plucky hollow]

short answer yes
longer answer: #1445764870564745228 message

[alpine minnow]

Idk why my notifications went off now for it 😭

[plucky hollow]

huh weird, sent that ping yesterday not now, silly discord

[paper cedar]

Urgh

BTW

Creative Tab Event also gets emitted before components are bound 😄

[lethal onyx]

sounds bad

[paper cedar]

I guess it doesnt really matter, but in our test we create an ItemStack eagerly for the tab icon

all that does need to be lazy now

java.lang.IllegalStateException: Missing element ResourceKey[minecraft:jukebox_song / music_disc_test:test_song]
hm.

[mental thorn]

Here's where binding components happens:
https://github.com/neoforged/Snowman/blob/dev/src/main/java/net/minecraft/server/MinecraftServer.java#L1593
https://github.com/neoforged/Snowman/blob/dev/src/main/java/net/minecraft/server/WorldLoader.java#L80

[paper cedar]

Yes, vanilla does it after resource reload, but we don't load resources in datagen

But no the binding happens earlier, actually. in net.minecraft.server.ReloadableServerResources#loadResources

Yeah I am not really sure we can bind components in datagen.

I.e. we have a test that does this:

private static final ResourceKey<JukeboxSong> TEST_SONG = ResourceKey.create(
        Registries.JUKEBOX_SONG, Identifier.fromNamespaceAndPath(MOD_ID, "test_song"));
private static final DeferredItem<Item> TEST_MUSIC_DISC = ITEMS.registerItem("test_music_disc",
        props -> new Item(props.stacksTo(1).rarity(Rarity.EPIC).jukeboxPlayable(TEST_SONG)));

That fails with the aforementioned error, since music disk data jsons aren't loaded.

The consequence of not binding components is that you cannot use ItemStack during datagen at all

[mental thorn]

Check where https://github.com/neoforged/Snowman/blob/dev/src/main/java/net/minecraft/core/component/DataComponentInitializers.java#L136 is called

[paper cedar]

we're already failing on #build() 😄

But yes, you're right that #apply() would then need to be called

but we can't even build the initializers

[paper cedar]

@wintry flower I think DataComponentIngredient will have to be changed to use ItemStackTemplate

[wintry flower]

Why not I guess

That seems in line with how components are used in recipe results

[paper cedar]

I am unsure what to do about the predicate though

Converting it to use a List of IST is easy enough

We cannot construct a DataComponentExactPredicate from an Item though, without realizing the actual item

which is only possible after datapack load

@wintry flower do you remember why you exposed the DataComponentExactPredicate from DataComponentIngredient instead of exposing DataComponentPredicate?

[wintry flower]

Can't you choose if the ingredient is strict vs non-strict?

[wintry flower]

Aren't ingredients part of datapack loading?

[paper cedar]

Datagen.

You'll create them programatically during datagen

[wintry flower]

Hmmm, no way to make them available during datagen?

E.g. using the registries future

[paper cedar]

That's what I did, but see here: #1445764870564745228 message

Your built-in components can ref content from datapacks

Which will be realized at that point in time -> crash

You can get around this by datagenning that particular default component

But we should be careful here since this might just be the path to datagen items/blocks.
Edit: The path being not using ItemStack outside of a level/gameplay

[paper cedar]

Hm @wintry flower so semantic question:
If a mod creates a data component ingredient with strict=true and gives it a Stick.
I think the current CODEC will serialize {item=stick, strict=true, components=[]}, yes?

So...... IF someone now adds a default component to STICK, that ingredient is not going to match sticks anymore, right?

As far as I can tell that is its current behavior

Is strict=true even useful anymore in a data component world?

[wintry flower]

I'm not sure that strict was ever useful

[paper cedar]

IMHO it makes more sense to check that the DataComponentPatch matches

And to only serialize that

[crude wadi]

It's not possible to add DataComponent conditions to the data component ingredient, is it?

[paper cedar]

It's using a DataComponentExactPredicate

Or do you mean something else? 🤔

So for the simple case of making an ingredient that matches a given ItemStackTemplate, I'd check the Item holder as we currently do.
From the component patch, we could create a predicate as follows:

private static DataComponentPredicate toPredicate(DataComponentPatch patch) {
  return getter -> {
    for (var entry : patch.entrySet()) {
      var type = entry.getKey();
      var value = entry.getValue();
      if (value.isEmpty() && getter.has(type) || value.isPresent() && !value.get().equals(getter.get(type))) {
        return false; // One of the patch entries doesn't match
      }
    }
    return true; // Empty patch always matches
  };
}

[robust pike]

As a heads up, I’ll be done with the primer over the weekend. There are a lot of examples and explanations I need to write up

[paper cedar]

Alright, I made our test datagen at least work without using any ItemStack

[paper cedar]

Okay. I can get in-game now in snapshot-3

But the last few commits are definitely a bit messy<

[paper cedar]

@graceful laurel i had to kill https://github.com/neoforged/NeoForge/pull/1789 partially since ModelManager#apply ran before default components are known
But I did add a a marker to restore it

[paper cedar]

Alright, to re-implement SleepFinishedTimeEvent

I'd introduce a util class:

public sealed interface ClockAdjustment {
    record Absolute(long ticks) implements ClockAdjustment{}
    record Relative(long ticks) implements ClockAdjustment{}
    record Marker(ResourceKey<ClockTimeMarker> marker) implements ClockAdjustment {}
}

Store that in the event, with getter+setter

[paper cedar]

Here's the new SleepFinishedTimeEvent for the clock manager: https://github.com/neoforged/NeoForge/pull/2927/changes/562557e7e320646104908f150d5f995afd6469a7

[lethal onyx]

what does the time command do? does it also have the restriction of not going into the past?

[paper cedar]

No idea, the previous event had that restriction 😅

I suppose you're right we can remove that restriction if the vanilla time command allows it

There's no limit

I'll remove it from our event

Alright, made the adjustment

[round spear]

vanilla doesn't allow negative I think

maybe?

other than that no limits

[plucky hollow]

21.11 (vanilla) time command doesnt allow negative atleast
including the /time add sub command
which i would have thought negative would subtract time rather than add

[round spear]

They probably didn’t want to deal with ensuring it doesn’t become negative if it is larger than current time so just disallow it

[empty night]

Minecraft: I have negative time

Furnace: "well I'll just uncook that beef then...."

[paper cedar]

the new system allows setting the tick at the very least

without clamping

the new system also does NOT wrap

Hm. I am auditing all the farmland instanceof patches

We're replacing level.getBlockState(relative).isAir() with level.isEmptyBlock(relative) in one of them

Is there a functional difference?

[wintry flower]

iirc yes

[indigo venture]

light blocks?

[wintry flower]

I think bumblezone has some air-but-not-air blocks

[paper cedar]

Okay but patching that selectively there but not anywhere else

is kinda pointless, no?

Also, no, wait, even bumblezone will not have a custom Level subclass

And LevelReader just has this:

default boolean isEmptyBlock(BlockPos pos) {
    return this.getBlockState(pos).isAir();
}

So.... I think I'll yeet that patched line entirely

[wintry flower]

Hmmm, maybe ask @runic anchor

[paper cedar]

@graceful laurel thoughts on using this to validate for missing item models?

// Eagerly report missing item models
NeoForge.EVENT_BUS.addListener(ClientResourceLoadFinishedEvent.class, _ -> {
    for (var item : BuiltInRegistries.ITEM) {
        var modelId = item.components().get(DataComponents.ITEM_MODEL);
        if (modelId != null) {
            // This will internally warn about each missing model at most once
            Minecraft.getInstance().getModelManager().getItemModel(modelId);
        }
    }
});

[graceful laurel]

Looks reasonable. Though it seems weird that components would be bound by that point since IIRC datapacks aren't loaded on the client until you join a world or server 🤔

[paper cedar]

Hmmmm

Have not tested it yet 😄

[bold forge]

first use of _ in neo?

[paper cedar]

not first I think

[paper cedar]

Yup absolutely right, as nice as it looks. Doesn't work 😄

[graceful laurel]

My only other idea would be to use TagsUpdatedEvent on the client to do the check. By that point I would expect components to be bound

Could even add a flag so it only happens on the first world/server join

[paper cedar]

Change of plans 😄

new event:

public class DefaultComponentsUpdatedEvent extends Event {
    private final ResourceKey<? extends Registry<?>> registry;

    @ApiStatus.Internal
    public DefaultComponentsUpdatedEvent(ResourceKey<? extends Registry<?>> registry) {
        this.registry = registry;
    }

    /**
     * {@return the key of the registry whose default components have updated}
     */
    public ResourceKey<? extends Registry<?>> getRegistry() {
        return registry;
    }
}

I'd emit that from net.minecraft.core.component.DataComponentInitializers.PendingComponents#apply

Yeah that seems to work

[graceful laurel]

👌

[paper cedar]

It'll emit too when you open the create world screen

but whatever, the warnings will only show once until you reload client resources and reset the tracked set in the ModelManager

IDK if this was previously possible, but a server datapack can now change the default item-model component of pretty much anything + send a server-sent resourcepack 😄

[graceful laurel]

That's been possible since the introduction of the item model data component, it's kinda the whole point of that component

[graceful laurel]

At least for this check that's actually pretty useful because it keeps the resulting messages away from the majority of server startup log spam

[paper cedar]

Yes but not changing the default model of stuff?

[graceful laurel]

Ah, no. I missed that you meant the default one
Though, are you sure a datapack can set the default value? I've only looked at the code in Snowman but to me it seems like the component initializers are entirely code-controlled

[paper cedar]

Hm. I thought that was the point of this exercise 😄

Mhmhmhm maybe I do misunderstand why they're doing all this dance

Oooor, that is coming in a future snapshot

Well anyhow: https://github.com/neoforged/NeoForge/pull/2927/changes/4724b5485306996b7fc0cff9598ff5d76cb3e679
Introduced new event, and used it to restore both item model validation, and also move default resource invalidation to a better place

[graceful laurel]

I think you forgot to generate patches

[paper cedar]

argh probably

[paper cedar]

amended the commit

https://github.com/neoforged/NeoForge/pull/2927/changes/b8f15eb95a164f5794a3cf49d7178762a6d97394#diff-d7c154e9e73102ebeab0ed87e282edb4ba11704932fc65c9228137878558136e

I think soon the -snapshot-3 port shall be good enough to merge

The only thing to check I think is my impl of the variable speed clocks

Tech reminded me, we have a connection-type sensitive StreamCodec

that could probably be used to simplify the syncing, I currently have written entirely separate records for syncing

This stuff: https://github.com/neoforged/NeoForge/blob/port/26.1-snapshot-3/src/main/java/net/neoforged/neoforge/network/payload/ExtendedClockState.java

[runic anchor]

Keep

Heavy Air and Windy Air are air blocks and should have isAir return true but Mc needs to be patched so that a chunk made of only isAir true blocks doesn’t wipe out all the custom isAir blocks and make them Air.

You can see this with Cave Air. Make a chunk section be entirely Cave Air and the game will see it as “empty” and swap the blocks with Air

[visual basin]

we're talking about a patch in farmland specifically

[runic anchor]

https://github.com/neoforged/NeoForge/pull/471

[visual basin]

yeah but that's unrelated

[runic anchor]

Ah didn’t ready farther up

As long as it ultimately uses isAir, that’s what is needed

[visual basin]

relevant points

A)

Hm. I am auditing all the farmland instanceof patches
We're replacing level.getBlockState(relative).isAir() with level.isEmptyBlock(relative) in one of them
Is there a functional difference?

B)

And LevelReader just has this:

default boolean isEmptyBlock(BlockPos pos) {
return this.getBlockState(pos).isAir();
}

[runic anchor]

Does branch have runnable datagen now

[paper cedar]

yes

[runic anchor]

I’ll do spear tag rename now then

[paper cedar]

and yeah, I was confused by LevelReader#isEmptyBlock. You can't really override that as a mod anyway

[graceful laurel]

The default item resource reset made me realize something: static registries share a single holder across sides in singleplayer and data components referencing datapack objects now hold the actual objects instead of just their key in the prototype (previously the EitherHolder would hold the key in the prototype and the real value only if appended via component patch). This means that the prototype now leaks datapack objects across the thread boundary in SP, specifically the client-side ones if the initializers run on both threads in SP or the server-side ones if they only run on the server

[paper cedar]

yes but is that specifically a problem?

[jagged ridge]

our patch to how equals works on holders bandages over some of the problems iirc but creates other ones

[visual basin]

if you include enchantments in the default datacomponents then one side could have unregistered enchantment objects

[graceful laurel]

Yes, because datapack objects are synced even in SP which means the server and client have entirely different instances of these objects which causes serialization errors since value->key lookups are reference-based

[visual basin]

might need a disclaimer like "don't include datapack-registrables in the default components"

[graceful laurel]

Vanilla itself is doing that so that's already moot

[visual basin]

can we proc any funny vanilla bugs from that

[graceful laurel]

Why else would the whole EitherHolder thing have existed? 😄

[jagged ridge]

It seems like they should conceptually not be storing datapack-driven information in the static registry at all

E.g. component map prototypes should be part of the reloadable resources object (e.g. ReloadableServerResources if they haven't renamed it since my time)

As soon as you attach any sided data to a static registry there will be problems

The other option would be to ditch global static registries and instead build one on each side at datapack reload/sync time, but this will double memory usage (it will probably happen eventually though...)

[paper cedar]

Ok but they have to figure this out too?

Because then, I am not too worried 😄

[jagged ridge]

not necessarily

they could get lucky

[wintry flower]

Maybe ask a mojang dev

[paper cedar]

Don't bug them on weekends 😛

I'll be honest, I dont do enough with datapacks/registries to fully grasp the extent of the problem

If I understand you correctly, i.e. if an item defaults to having enchantment X

and you have some client-side code that tries to send a packet to the server referencing that enchantment

[jagged ridge]

The problem is that object identity of deserialized datapack objects matters on each side, but the way Mojang designed the system makes it extremely easy to leak the objects from one side to the other

[paper cedar]

but that is only the case if integer Id based serialization is used?

[jagged ridge]

I think it also matters if location-based serialization is used

[graceful laurel]

The serialization type is irrelevant, any value->key lookup fails

[visual basin]

what vanilla objects have default components containing datapack registrables?

[wintry flower]

This looks like an oversight, but they might already be aware of it

[jagged ridge]

enchanted books?

[visual basin]

okay, great, will see if I can make vanilla bugs happen with that once there's a neofroge for the snapshot

[paper cedar]

Ok but how did this work previously?

[graceful laurel]

Spawn eggs, goat horns, enchanted books and a couple others

[paper cedar]

given there was only a single Item instance

across both sides

[wintry flower]

ResourceKey

[graceful laurel]

Through EitherHolder. In the prototype it would hold the key to resolve at time-of-use, in the patch it would hold the actual value

[jagged ridge]

I believe there were already isolated crashes or other weirdness in 1.21.1 from mods leaking the enchantment objects across sides by accident

[visual basin]

e.g. Sapling Blocks are constructed with a ResourceKey referring to the placedfeature of the tree to generate (so did goat horns)

[paper cedar]

And now they dont use a resourcekey anymore?

[graceful laurel]

Now the initializer specifies a key and the component binding resolves that key to the datapack object

[paper cedar]

we can just ask about it on monday

[visual basin]

when is "component binding resolves"

[graceful laurel]

After datapacks are loaded

[paper cedar]

everytime datapack reloads or client receives synced datapack

[visual basin]

oh perfect that should work without problems then

the only datapack registries the client has are the ones it receives from servers

[paper cedar]

yes but the item instance is shared

however, the components are in the holder now

[visual basin]

oh hmm

that's weird

[graceful laurel]

And said holder is also shared since it's owned by the item instance (because intrusive) or the static registry in the case of non-intrusive holders

[paper cedar]

yes it is now

but it might be prep

[visual basin]

I wonder though

if they intend for static registrables to be accessed through the client registryaccess

[jagged ridge]

Hasn't that been possible for a while?

[visual basin]

yes

but nobody does it

[visual basin]

which could conceivably cheese the holders to use the components of the correct side

mm the intrusive holders would prevent that from working

are those still a thing?

[graceful laurel]

I'm pretty sure that registry access just references the static registry itself

[graceful laurel]

Yes

[visual basin]

okay it's not that then

I'll wait for the public neofroge build and poke around then

[graceful laurel]

Or just share the registry access between server and client and nuke the entire issue once and for all. Since datapack registries are not reloadable, they are effectively immutable once the server is started, so there's (at least in my mind) zero benefit to serializing them, sending them across the thread boundary and re-building the entire registry from scratch on the client

[paper cedar]

alright i just need to tackle the clockmanager and then we can merge the snapshot-3 branch

[jagged ridge]

gets mixin ideas for 1.21.1

[graceful laurel]

[paper cedar]

the objects in it are also immutable to my knowledge

meaning thread-sharing should not be a problem

[graceful laurel]

Yup

[paper cedar]

Alrighty

Vanilla has

public record ClockState(long totalTicks, boolean paused) {

• CODEC + STREAM_CODEC

We kinda need to add float speed, float partialTick to that

I created a completely new record for that, but as far as I understand, we can just extend the CODEC and use that connection-type aware STREAM_CODEC we have, to get the same result, yes?

[graceful laurel]

Yeah, that should work just fine. The new fields just need to be optional in the DFU codec

[paper cedar]

Good point, I'd have forgotten about that.

That should do the trick then, these fields are easily defaulted

Codec.FLOAT.optionalFieldOf("fractional_tick", 0f).forGetter(ClockState::fractionalTick),
Codec.FLOAT.optionalFieldOf("speed", 1.0f).forGetter(ClockState::speed)

Yeah that makes this patch a WHOLE LOT easier

I think we can drop the entire custom time sync packet

[visual basin]

is that synced? how does the streamcodec deal with extra fields for neoforge server -> vanilla client

[graceful laurel]

We have a conditional stream codec that switches between two codecs depending on the connection type

[visual basin]

ooh neat

[paper cedar]

I stopped caring about reformatting this block in the patch since it'd conflict whatever we do (given the original is just one line)

    // Neo: sync the extended clock state only to NeoForge clients
    public static final StreamCodec<net.minecraft.network.RegistryFriendlyByteBuf, ClockState> STREAM_CODEC = net.neoforged.neoforge.network.codec.NeoForgeStreamCodecs.connectionAware(
            StreamCodec.composite(
                    ByteBufCodecs.VAR_LONG, ClockState::totalTicks,
                    ByteBufCodecs.BOOL, ClockState::paused,
                    ByteBufCodecs.FLOAT, ClockState::fractionalTick,
                    ByteBufCodecs.FLOAT, ClockState::speed,
                    ClockState::new
            ),
            StreamCodec.composite(
                    ByteBufCodecs.VAR_LONG, ClockState::totalTicks,
                    ByteBufCodecs.BOOL, ClockState::paused, ClockState::new
            )
    );

Might be interesting to make a version of that for more easily tacking on stuff

Without repeating it

We might be able to improve it further by removing our custom time command and adding the new stuff to mojangs

but anyhow, nice to be able to simplify this for now

BTW, to answer the point from earlier about turning back the clock

Aight, quick test shows it syncs+persists fine. So gogogo

Oh, XFact oops, i clicked re-request review, but I don't actually expect a review

I'll merge it when the CI/CD passes

[graceful laurel]

I'm doing a quick look over it right now anyway 😄

[paper cedar]

Yes, we'll also have the PR as record of what changed for snapshot-3

Hm, although. No rush, I can just merge it in a few hours

[graceful laurel]

One thing I already noticed: is there actually any benefit to patching in shaped and shapeless recipe builder factory methods that take an ItemInstance since stacks aren't usable in datagen anyway?

[paper cedar]

No, true

I have a feeling we have more places we expose ItemStack that will no longer work

[graceful laurel]

Probably, yeah

[runic anchor]

I tried to run datagen rip

[graceful laurel]

Did you run setup?

[runic anchor]

let me see. thought I did before

I get a patch failed to apply

[paper cedar]

Oh

I just merged port/26.1

Probably because of that

That also explains why suddenly ATs are not validating anymore

Uh what happened to EditGameRulesScreen

[graceful laurel]

It was split AFAIK

[paper cedar]

I'll revert the merge for you grunt, gimme a sec

I can only fix it later

Ok, I force pushed the branch back to the commit before the merge. That one should setup/work fine

I'll handle that later

[runic anchor]

Large tag cleanup pushed to remove all legacy tag stuff and fixed spears/tridents
https://github.com/neoforged/NeoForge/pull/2927/commits/88b68c75dd0cf429f8f582c4b5cf6425ecf64b4c

I'm done now lol

[robust pike]

Take your primer: https://github.com/neoforged/.github/pull/39/commits/538dcc93b52f393aabf9ebc65363fd9ee51bd620

Data component initializers, stack templates, world clocks, platnable tags, and the Entity#interactAt removal.

[rain herald]

hmm

interesting

loving data driven VillagerTrades

but it seems to me

VillagerProfession could also be easily data driven

at least, if the Predicate<Holder<PoiType>> becomes a HolderSet

which it already is basically, they only check tags

actually fuck it make PoiType a registry data driven too!

it seems to use Set<BlockState> unnecessarily

cuz they just do getPossibleStates for everything

the entire villager pipeline could be data driven that would be sick

even VillagerType thats just empty

[rain herald]

also lowk the advance_time gamerule is no longer needed since u can just pause the overworld clock

[paper cedar]

the gamerule applies to all clocks

[rain herald]

hmm ok

interesting

[paper cedar]

and it will do so when the world is created immediately

[rain herald]

is there a reason gamerules arent prefixed with minecraft:?

that is a weird inconsistency

i wouldnt be opposed to command suggestions ommiting the namespace when default

[graceful laurel]

PoiType is and has always been a registry

[rain herald]

data driven registry i mean

[rain herald]

anyways on this, it would be a fine change but i would like it consistent, its weird to have it ommitted for just gamerules while u now have to do /time set minecraft:day

[graceful laurel]

That would be potentially dangerous as every blockstate can only have exactly one PoI

[rain herald]

i mean PoiType just takes in a Set<BlockState>

u could perform the exact same validation when data driving

when registering a POI Type currently, it calls this method

private static void registerBlockStates(final Holder<PoiType> type, final Set<BlockState> matchingStates) {
        matchingStates.forEach(blockState -> {
            Holder<PoiType> previous = (Holder<PoiType>)TYPE_BY_STATE.put(blockState, type);
            if (previous != null) {
                throw (IllegalStateException)Util.pauseInIde(new IllegalStateException(String.format(Locale.ROOT, "%s is defined in more than one PoI type", blockState)));
            }
        });
    }```

it being blockstate specific isnt actually used

they could make it refer to a Block, perform the same check when loading data driven entries, and fail if there are duplicates (reporting the duplicates for packs to know)

[indigo venture]

the but report for full locations not working was resolved as fixed
#1425127445803045186 message
its just the suggestion handler doesnt suggest them

[rain herald]

GameRuleCommand is... something

i feel this couldve been fixed nicer but sure

this is so weird how they did this

i feel like they couldve treated it like a normal registry and it wouldve been fine

[indigo venture]

nope, they cant

had a talk to one of the mojang devs about that

brigadier doesnt support 'dispatch' style arguments, where the acceptable values for later arguments depends on the value of an earlier argument

but that is required here to have different acceptable values (bool or int) depending on the previous value.
they hack around it by making all of the literals, but that has the consequence of all of this mess

[rain herald]

they couldve at least changed game rules to have a ResourceLocation id instead of a string one

to make it a bit more consistent

instead of flattening the registry id down and suggesting that and then adding a fully qualified version

[craggy river]

Out of curiosity, what's the reason a blockstate can't be marked as more than one type of POI?

[jagged ridge]

The game has a Map<BlockState, PoiType> internally and the rest of the code makes that assumption

[craggy river]

Huh. I wonder why they did that? It might be useful to have made it a Map<BlockState, Set<PoiType>>, that way there wouldn't be problems with adding new PoiTypes in the future.

[rain herald]

they could remove that limit

but then when a villager goes up to a block to get a job which one does it pick

could be random ig

like trades themselves

[plucky hollow]

i mean if you rly want multiple poi per Block that is possible
since the system is BlockState bound not Block
add a property to your block which defines which PoiType it should support

similar to what beds and the HOME poi currently does
where only the HEAD part of beds have the poi registered

but id also recommend against this as property spam on your block states can be a problem

[plucky hollow]

https://www.youtube.com/watch?v=ILnYqYftvX4

[plucky hollow]

oh snapshot 3 merged welp i know what im doing for the reset of the night

[visual basin]

F5ing the neoforged homepage to see if the new build is up?

[plucky hollow]

i mean the pr build published you could use that

until actual release

[visual basin]

I did that once before but I forgot how

is the maven where the builds are linked to from anywhere?

[plucky hollow]

its in the pr publishing comment

https://github.com/neoforged/NeoForge/pull/2879#issuecomment-3663455382

just need to expand the spoiler

[visual basin]

ah I was looking at a different pull request https://github.com/neoforged/NeoForge/pull/2927

[visual basin]

okay earlier I wanted to look at the new thing whereby items' default components are stored in their holders and how concerning that is in a server/client environment

I see something fucky being done via RegistryDataCollector.updateComponents which happens when the ClientboundFinishConfigurationPacket is handled

[plucky hollow]

yeah components are only bound after datapack reload now afaik
which is all handled via the component initalizer system

this means until then you can not use components or anthing else which interacts with them until datapacks have loaded
(no itemstacks in datagen, use the new stack template instead)

[visual basin]

right right but the concern was, if they're stored in static registry holders, how does that work in singleplayer

where both sides share the same recipes but components might hold datapack registrables which have distinct and non-interchangeable values between server and client

[plucky hollow]

that im not 100% on will have to ask @paper cedar whos spent the most time digging into thesystem so far iirc

but iirc there was some chat abou components leaking sides at one point cause of this new component holder registry shenenniagans

[visual basin]

okay, when goat horns declare their instrument, they define a lambda

which getOrThrows some instrument from the provided context

that might be the secret sauce

yeah they use delayedComponent as opposed to component like tools do

if the lambda isn't evaluated until a sided registry is available then there wouldn't be any problems

[plucky hollow]

ahhhh why does ItemStackTemplate take in Item as its ctor args and not ItemLike
like how ItemStack did i cant simply pass my deferre item into the ctor 🙁 i have to .asItem or .value

i wanna do new ItemStackTemplate(MyItems.ITEM, 5) but i gotta do ``new ItemStackTemplate(MyItems.ITEM.value(), 5)`

mojang please change that

so much of my staff takes ItemLike everywhere cause it makes passing holder/item much easier

[jagged ridge]

It sounds like we could pretty trivially patch that in if it's not added in vanilla by the full release

[plucky hollow]

wouldnt it cause weird conflict issues with the existing Item ctors?

or maybe you mean swap out the Item for ItemLike in patch

ahhhh my ghost rendering got borked yay, worked perfectly fine in snapshot-2
what changed

swapping from using AfterTranslucentBlocks and AfterEntities to rendering both static models and block entities during AfterOpaqueBlocks fixed it, weird, did anything change in how the pose stack is constructed/passed to the RLSE?

[paper cedar]

The ItemLike constructor is sth we can just patch in

[plucky hollow]

true but having things like this in vanilla would be better

[paper cedar]

also wait, shouldn't the deferred register be a Holder<Item>?

Ah, I see, there's no shorter overload

[plucky hollow]

yeah but theres only 1 ctor for it which takes all 3 args

[paper cedar]

So here's the thing: we should probably not just add ItemLike overloads

since that just leads to ambiguous calls

We can replace the vanilla ctor

[plucky hollow]

yeah thats what i was thinking
#1445764870564745228 message

wouldnt replacing vanilla also cause issies with multiloader mods?

if anything we add static from(ItemLike, [count], [patch]) or something?

[paper cedar]

Mixins into thoses methods have issues

But source-wise it shouldn't really make a difference

You can't realistically share compiled classes either

[plucky hollow]

i cant think of why you would want mixin into the ctors so should be fine
but im sure someone will find some excuse to mixin them

they all point to the main holder ctor anyways so people can mixin that if need be

[plucky hollow]

tbh the ctors the template has are intresting, its missing a few for the full set of overloads
Item, int, DataComponentPatch
Holder<Item>
Holder<Item>, int
Holder<Item>, DataComponentPatch
those last 3 would mean the ItemLike isnt as necessery since would go down the holder route

[fresh gyro]

Sharing compiled classes is trivially doable, by simply compiling against something like NeoForm, has worked for years and is used for years.

We should not remove vanilla methods, but provide overloads with deprecation warning, like we have always done.

The question is does this break binary compile if the signature is changed

[indigo venture]

is used for years
how?? loaders have used different runtime mappings for almost all of that time

[fresh gyro]

By remapping the common compiled classes

I have been doing that for at least 2,5 years

And I was not the first as I borrowed that technique I think from Arch

[paper cedar]

That does not work if you change signature of vanilla methods

[fresh gyro]

Which we generally don't do......

We always provide an override with a @Deprecated

At least I am not aware of one

Especially not one in such a widely used class

[paper cedar]

Please look at the discussion you replied to 😛

That is exactly what was being discussed

[fresh gyro]

I am fine with it

I was not discussing otherwise

Just stating that using common compiled classes is very much widespread

And hence us needing to be cautious

[lucid tree]

Do be careful xD

I rely on common sources

[fresh gyro]

Yeah basically any multiplatform mod does

[plucky hollow]

how would we feel about neo introducing a template builder class? gets around this issue and is what ive been using in my mods

-# ignore the itm = item.asItem i was gonna validate not air but ctor does that, just forgot reinline the var

[paper cedar]

Hmmm idk maybe?

[paper cedar]

We can do #of or we can replace the ctor, or we may add a specific overload for DeferredItem, but that might also be ambiguous, not sure

[plucky hollow]

deferred item would be the worst out of the bunch imo
since people including myself do their own custom deferred holder types

[paper cedar]

Same (as in, I do too)

[paper cedar]

@plucky hollow as a quick one, I'd just add of()? Or..... we just add the convenience overloads for HOlder<Item>

that will also work for our crap

[plucky hollow]

i wanna say add the overloads #1445764870564745228 message
to stick with how vanilla defines them, having random of methods feels out of place/clutter to me

[paper cedar]

ye

[wintry flower]

Overloads will create ambiguity

And will not solve the problem of ItemLike

I think of with ItemLike makes the most sense

[visual basin]

why not just patch the constructor? we've done it before

[plucky hollow]

unless im missing something, no it wont? this works just fine

[plucky hollow]

multiloader compat mostly iirc we cant swap Item to ItemLike cause then the signature is different

[visual basin]

do we care about multiloader compat

[wintry flower]

This won't work for a generic ItemLike

[plucky hollow]

we are talking about patching in the holder overloads to not need the ItemLike overloads
with these our deferred items will go through the holder ctors rather than the item

[wintry flower]

I guess that works

[plucky hollow]

its also more correct for how the template works
doesnt need to resolve the item just to then go and grab the holder futher down the chain

[high sonnet]

tomorrow...

[paper cedar]

Yeah it's Item and Holder<Item> overloads, which IMHO is fine

given Vanilla has both, but only uses Holder<Item> for its "canonical" constructor

For us it works nicely due to DeferredItem being Holder<Item>

[paper cedar]

you know you also just could have pushed the overloads to the branch 😄

don't be shy ^^

@plucky hollow https://github.com/neoforged/NeoForge/commit/b38a28e6b01224e89395bea9e644c0b24e32eb69

[graceful laurel]

I haven't tested yet whether this actually causes serialization errors but my suspicion of the component binding causing datapack objects to be leaked across sides is correct. Running the following snippet:

List.of(
    Items.EGG.components().get(DataComponents.CHICKEN_VARIANT).value(),
    ServerLifecycleHooks.getCurrentServer().registryAccess().getOrThrow(ChickenVariants.TEMPERATE).value(),
    Minecraft.getInstance().level.registryAccess().getOrThrow(ChickenVariants.TEMPERATE).value()
);

returns the result shown in the screenshot which indicates that the server's datapack objects are leaked to the client thread

Further testing: Serialization of a stack of an item holding this leaked object in its default components works fine because default components are never serialized. However, if you retrieve and then try to serialize the component value with the client level's registries (ChickenVariant.CODEC.encodeStart(Minecraft.getInstance().level.registryAccess().createSerializationContext(JsonOps.INSTANCE), Items.EGG.getDefaultInstance().get(DataComponents.CHICKEN_VARIANT))) then it blows up with the usual "X is not valid in current registry set" error.

[graceful laurel]

I ported FramedBlocks to snapshot 3 last night and I'm running into an issue with the component binding stuff, specifically its timing: I have a system that collects data from three sources, those being a set of default entries computed from the block registry, an event for code-driven registration of potentially complex entries and a datamap to provide simple entries. The computation of the default entries relies on being able to create stacks and therefore requires the default components to be bound. Until now this system used DataMapsUpdatedEvent to collect said data. However, now I'm running into the issue where that event fires before default components are bound on the server. Switching to DefaultComponentsUpdatedEvent would fix this on the server but would break on the client when connected to a dedicated server because datamaps arrive on the client after default components are bound and DefaultComponentsUpdatedEvent does not provide info on whether it's called from the server-side reload or from the client-side sync arrival, making it impossible to switch between the two events depending on the side. I think moving the DefaultComponentsUpdatedEvent dispatch into the newComponents.forEach() lambda (would have to be converted from a method ref) in ReloadableServerResources#updateComponentsAndStaticRegistryTags() and the lambda in RegistryDataCollector#updateComponents() and adding a reason to the event would be the best solution.

[paper cedar]

we can do that

i also wonder if it even needs to be emitted per registry

we could technically just do it once after all

[graceful laurel]

That's a thought I also had, yeah

[empty night]

Can someone more familiar than me either spot check or tell me where the Neo hooks for cutout models live? Some of my models are rendering super weird and I'm looking to debug

this white shouldn't exist, it's a cutout model (26.1-s2)

[wintry flower]

the Neo hooks for cutout models
no idea what you mean

[paper cedar]

render-type property in the model?

wasn't that a vanilla thing now?

[paper cedar]

hm that might just be another borked patch yes

should be somewhere in BlockModelElement (? or similarly named9

[empty night]

Aight, lemme dig around a bit more here

Might be on my part too, model datagen changed pretty hard

Yeah okay, it was culling changes. Got it.

[wintry flower]

I don't think so

yeah clearly not, see SimpleModelWrapper where we patch it in

[graceful laurel]

After further consideration, this would actually be beneficial for my use case

[paper cedar]

Yeah same

i.e. if you depend on multiple registries

[graceful laurel]

Yup

Btw, we should also note on the event that (at least as far as I can tell, need to check that this is actually the case) the component binding happens on every reload on the server but only on the initial datapack registry sync on the client (ignoring the potential of kicking a client back into config phase of course)

[paper cedar]

Hm doesnt it already say that á la "when joining a world"?

Do we want to record which registries got bound or is that poitnless

[graceful laurel]

Not sure that's useful info

[paper cedar]

@graceful laurel https://github.com/neoforged/NeoForge/pull/2933/changes

The event body is pretty much copy pasta of the tag update event since it has similar concerns (it does, right?)

Can you add your reasoning you outliend above to the PR description?

[graceful laurel]

Yup, just make sure you're actually using the new event's cause enum in the event instead of the tag event's enum 😄

[graceful laurel]

Will do 👍

[paper cedar]

Did I do that bad of a copy job? 😄

[graceful laurel]

You did

[paper cedar]

yes i did 😄

Didnt really update the docs either

Well anyhow, just check if the "style" of this event looks good to you

[graceful laurel]

Looks good to me

[wintry flower]

hey @signal juniper, is this something you are interested in upstreaming? (assuming you took care of the items-to-atlas rendering) https://github.com/neoforged/NeoForge/commit/84d77559be005a67275425bb4c427aebaf2bc513
I suppose you might not need it for your purposes but it looks easy enough and fairly harmless 😄

[graceful laurel]

They will need to fix it because item models using the light_emission field in the JSON are broken as well

[wintry flower]

oh this problem is exclusive to snapshots

[paper cedar]

OK snapshot build is out: 26.1.0.0-alpha.7+snapshot-3

[wintry flower]

java: incompatible types: net.minecraft.world.item.ItemStack cannot be converted to net.minecraft.world.item.ItemStackTemplate alright that sounds like a problem for another day 😁

hmm, @Nullable ItemStackTemplate is cursed 😄

who wrote ItemInstanceExtensions... why is there a plural there? 😅

do we want FluidStack to read the prototype of Fluids? 😄

hmmm

this is quite problematic:

    /**
     * {@return an ItemStackTemplate equivalent to this item instance}
     */
    default ItemStackTemplate toTemplate() {
        throw new UnsupportedOperationException();
    }

it will just fail on other implementations of ItemInstance

[rain herald]

will we be getting FluidStackTemplate and similar

[wintry flower]

probably yes

[graceful laurel]

I had a similar thought, yes. I would say we should do that

[wintry flower]

I just pushed a removal of ItemInstanceExtensions, don't be too sad 😄

since an ItemInstance cannot list all its components (if you stick to the interface) it doesn't make sense to turn it into an ItemStackTemplate

[paper cedar]

I was waiting for you to do that lol. My humble toTemplate 😭

But yes, the more we just use ItemStackTemplate anyway it becomes less and less useful

However, I might actually just reintroduce something like that, if it turns out during some ports that it is useful 😛

It is mostly when you work with ItemInstance as a consumer and have to pass it onto something that forces you to pass a template

[wintry flower]

technically you cannot list the components of an ItemInstance

(well I suppose you could iterate the component registry 😄 )

[paper cedar]

Yes but why would that matter

[wintry flower]

it makes the default implementation awkward and it's a bit of a red flag I guess

[wintry flower]

but I could see something like that happening

ideally you'd only call ItemInstance things when you have an item instance, but you might sometimes be forced to materialize it into a stack or template I suppose

[paper cedar]

Or, you know, we just fail 😄

[wintry flower]

a bit annoying for custom impls 😄

[paper cedar]

I dont see a use-case for custom impls, to be honest 😛

[wintry flower]

overall not a lot yet

wtf 😄

[paper cedar]

I'd expect GuiGraphics to actually take an ItemInstance for example

[wintry flower]

ah that's the decompiler

[paper cedar]

decompiler artifact?

yeah

[wintry flower]

VF:

CG:

[paper cedar]

mhmhmhm gotta nag covers 😄

[wintry flower]

looks cleaner, especially around

            } else {
                return this.mainhand.isPresent() && !this.mainhand.get().test((ItemInstance)livingEntity.getItemBySlot(EquipmentSlot.MAINHAND))
                    ? false
                    : !this.offhand.isPresent() || this.offhand.get().test((ItemInstance)livingEntity.getItemBySlot(EquipmentSlot.OFFHAND));
            }

[paper cedar]

@wintry flower GuideME 26.1.5-alpha is now out for snapshot-3

[wintry flower]

very nice, thanks 😄

[paper cedar]

I don't expect ae2 to be that quick 😄

[jagged ridge]

this really feels like it should be an EnumMap instead of those if checks lol

[wintry flower]

probably yes

[paper cedar]

Hrmpf, the name is now only accessible from an ItemStack apparently, which is quite annoying

[plucky hollow]

tbh wasnt sure if we had agreed on if the ctor overloads was the way we were going
and the way you worded this sounded to me like you were working/going to work on it
#1445764870564745228 message

[bitter gorge]

Interesting.. I've made chained extensions myself before that work just fine. Does IItmStackExtension extend ItemInstanceExtension? Or are the types converging on ItemStack?

I know i've had self methods in both extension interfaces, and had one extend the other without issue

[graceful laurel]

That error is just IDEA being stupid. The method it's complaining about is private. Player shows the same error

[bitter gorge]

seems likely, i thought it was a bit suspect from the methods being private

[plucky hollow]

not rly it wasnt a private method, was a default method which should have maybe been private

[graceful laurel]

That should be irrelevant as one of the two methods is private

[bitter gorge]

oh, yeah, all self methods should be private to avoid exactly this

mm, still probably ij bug from one not being private

[plucky hollow]

might be worth a immaculate task to ensure extension self methods are private?

[west thorn]

a) Yes, we know about registry value leak, probably going to share actual types until we kill intrinsic holders
b) don't get cozy with ItemLike, long term it needs to go, since it provides raw Item

[lucid tree]

Is the end goal, data driven?

[west thorn]

no, we just like breaking stuff /j

[lucid tree]

xD Fair

[plucky hollow]

as we are aware with how much yall keep poking rendering in each snapshot

can we have 1 snapshot where you dont break all the things, just 1 please /j

[lucid tree]

Proceeds to break Ender Dragon, making it breathe fire instead

[inner creek]

time for ItemHolderHolder

[fresh gyro]

OptionalItemHolderHolderWithComponentsAndAsItemTemplate

Why not go all out?

[plucky hollow]

thats just ItemInstance or ItemStackTemplate for item with components and count

[inner creek]

ItemStance

but yeah I meant just a Supplier<Holder<Item>> to replace the ItemLike

[plucky hollow]

fine TypedInstance<Item> that can be simplified to () -> Items.STONE.builtInHolder() basiclly

[cursive lily]

Do release candidates count?

[lucid tree]

As long as you dont break Wolves

[plucky hollow]

instructions unclear wolves are now mobs and the pet aspect split into dog/puppies

[inner creek]

next snapshot: "we have introduced dogs to separate them from wolves because a tamed wolf is not the same as a dog"

I always found it interesting that they did this with cats vs ocelot, but not wolves

[plucky hollow]

is this a hint for todays snapshot ? 26.1-rc

[inner creek]

lol, that would make it the smallest drop ever, I think?

[plucky hollow]

but on the flip side the most adorable with the babies

so worth it

[mental thorn]

there have also been no pre releases yet

[lethal onyx]

ItemHolderLike?

[high sonnet]

Yall have a real will they won't they relationship with Mojang don't ya.....

As in will they fix this or do we have to

And will they break everything with the next snapshot or won't they

[plucky hollow]

there is no "will they break stuff next snapshot", only "they will break stuff next snapshot"

[west thorn]

you can break something for yourself or we will break it for you, the choice is yours /j

[paper cedar]

But yeah the argument about ItemLike makes sense. Wonder how the Block<->Item relationship will shape out in the code then

ItemHolderLike

[plucky hollow]

item component which holds a block id/holder, when present item turns into a BlockItem?

i mean isnt that the ultimate goal to essentially ECS all the things

also wonder if its worth us dropping ItemLike from our DeferredItem at some point
treat them more as what they are holders, not items

patch in more Holder<Type>/Supplier<Type> overloads in places to make passing (deferred) holders easier
and not need to .value() everywhere

but thats a thing for future maintainers, not now

[paper cedar]

It would also resolve the current annoyance that is the ItemStack ctor

[lethal onyx]

that way is already solved tho, the other way around is the problem, how does the block know what Item it "is"

[plucky hollow]

tbh why does the block need to know that info?
but that would prob also be a component once blocks get those

or what ever thing blocks end up getting that combine block entity and state into a componable system

[plucky hollow]

T-~1/2hr(s), what fun things does mojang have in store for us today

[hazy wind]

obfuscation is back, end worse then before /s

[inner creek]

44 minutes I would guess

[plucky hollow]

please no, im loving how readable the code is now, lets not go back

[rain herald]

back to java 8, reobf, no mappings, max proguard

[plucky hollow]

someones asking for a timeout /j

[untold oyster]

I think the devs would riot if someone attempted to push that

[rain herald]

mojang holds a lot of power

[untold oyster]

I know I will /j

[rain herald]

"if hytale is so great go mod that"

[plucky hollow]

java8 would be enough for me to give up modding let alone the rest of it

[rain herald]

I mean that one would be difficult they use a lot of modern java features

the rest of it is just changing some settings

[zenith dove]

the new LTS would be the last unfucked version of mc

[rain herald]

the concept of an LTS being a snapshot

we would forever be stuck on 26.1 snapshot 3

[zenith dove]

both the loaders can mod them now, why not lol

[rain herald]

it was all in secret preparation

[hazy wind]

Progaurd?
what about Microsoft Obf?

[untold oyster]

they're reimplementing Minecraft in Rust but still using the JDK /j

[lucid tree]

https://github.com/Pumpkin-MC/Pumpkin

:>

Already have a Server reimplementation in rust

[untold oyster]

I raise you https://dayssincelastrustmcserver.com/

11 so far listed

[lucid tree]

Damnn

Issue is, nobody wants to use these softwares.

Literaly nobody in the normal market will.

[plucky hollow]

oh so close to 1 year, can we get there before the next rust server

[untold oyster]

yea, these alternative implementations are for the niche parts of the community that either want them or need them

[lucid tree]

Ya

[untold oyster]

the one I had a more than passing interest in was MCHPRS, which is specifically meant for redstone computers

PicoLimbo's also interesting since it's meant as a lightweight 'limbo' server (i.e., servers to hold players as in a queue or something between other servers)

[plucky hollow]

<t:1768921200:R> - Soon™

[lucid tree]

<t:1768920637:F>

[untold oyster]

ready the frogs!

[lucid tree]

https://tenor.com/view/frog-ribbit-smiling-animated-gif-12261731

[wintry flower]

Nope 😉

[hazy wind]

I bring you this
https://www.youtube.com/watch?v=p-k5MPhBSjk

[inner creek]

huh no snapshots yet, I was wrong :O

"Sorry the whole team was busy playing Hytale, no snapshot today."

[plucky hollow]

dropped ~ <t:1768316520:t> last week iirc, so yeah either no snapshot or late?

[snow island]

I think when people said that Hytale would kill Minecraft - they meant it not like this

[plucky hollow]

"minecraft is now a hytale plugin" /j

[plucky hollow]

26.1 Snapshot 4

• Primer: https://github.com/ChampionAsh5357/neoforged-github/blob/update/26.1/primers/26.1/index.md
• Article: https://www.minecraft.net/en-us/article/minecraft-26-1-snapshot-4
• Changelog: https://misode.github.io/versions/?id=26.1-snapshot-4
• Notion: https://apexmodder.notion.site/2524f070f88880648482e3cbdcba566a?v=2524f070f88880b29f60000c52785ecb&p=2ee4f070f88880718a68d91f542a80e7&pm=c
• SnowMan: https://github.com/neoforged/Snowman/commit/a680b7c5a7fb3af094e1df0378a2f8098dd066d6

SlicedLimes Videos:

• Main: https://www.youtube.com/watch?v=Xhq6WsoUYbE
• Pack: https://www.youtube.com/watch?v=Vc88sfOxfkI

||<@&1067092163520909374>||

[quartz spade]

New version detected: 26.1-snapshot-4.

[compact swan]

an hour early

[rain herald]

smol 😪

im waiting for a big feature bomb

I figured more bebes but I thought there would be more

[plucky hollow]

uhhh i think snowman is stuck? https://github.com/neoforged/Snowman/actions/runs/21178317117/job/60913261199

[rain herald]

is snowman not just a diff

[quartz spade]

Minecraft 26.1 Snapshot 4

Earlier today we pranced into the shipping room in order to bring you the fourth snapshot of 26.1, featuring an equestrian batch of game drop cuteness. With this week's release, baby mounts are getting an adorable makeover! Donkeys. Mules. Every single hor...

Jan 20, 2026, 3:58 PM

[plucky hollow]

its private repo containing decompiled sources for all the versions
so yeah, mostly used to diff changes between versions

can be generated using Snowblower but note the current version does not work with recent mc versions, there is a pr being worked on tho for this

[inner creek]

Baby chicken wing texture has been updated/fixed
heck, the sticker is out of date now I guess

[plucky hollow]

must be updated stat

retriggered the snowman run and it got stcuk again so yeah i think the runner might be borked

[hazy wind]

https://tenor.com/view/horse-horses-owned-gif-21828610

[inner creek]

eh

did they change?

looks the same to me

[plucky hollow]

alot of entity brain ai stuff swapped out for new activity data

hmm

or well brain init methods swapped to new activity data

[rain herald]

brain overhaul?!

https://tenor.com/view/cynthia-erivo-shocked-shock-jumping-back-walking-back-gif-12035888169669287630

smartbrainlib please mojang my life is yours

[plucky hollow]

also DataGenerator got poked...

theres 2 new sub classes Cahced and Uncached

[rain herald]

perhaps a 0 pixel fix

[strong wing]

Texture wise they changed from a 2x2x1 pixels to a 2x2x2 pixels

[rain herald]

wat

[strong wing]

last snapshot on the top, new snapshot on the bottom

[untold cypress]

[strong wing]

might be the underside of the texture thinking about it

[untold cypress]

give me a github texture diff

[plucky hollow]

water mask pipeline is now translucent, kinda suprised it wasnt already tbh

ooh feature rendering got split, solid features are rendered first now then translucenet after

wait you can breed the zombie horse?

i mean i saw babys were added but undead breeding sounds weird

[strong wing]

likely a fix for https://bugs.mojang.com/browse/MC/issues/MC-302635

[plucky hollow]

also yeah brains got some kind of rework/overhaul
alot of stuff is now init'ing this new activity data class now rather than a brain/brain provider

so people with custom entity ai should prob think about updating to 26.1 if they havnt already

[hazy wind]

looks like 1 big one

[rain herald]

@clear zinc this is interesting

kinda like sbls brain activity group

[plucky hollow]

new slot range replaced old piglin/villager one it seems

[alpine minnow]

In the other pic the wing is straight but the sticker is not or is that because the wings move or something

[inner creek]

the screenshot for the sticker was taken when the chick was falling

so the legs were flapping and the feet were swinging

[alpine minnow]

Ah

[plucky hollow]

well thats all the things that jumped out at me
im off to go make some burgers,
if someone looks into the borked snowman run and fixes it please do ping me so i can update the pin

[paper cedar]

Started the update yet?

[plucky hollow]

im not aware of anyone starting the branch and im unsure on the steps needed to do it myself
also my plans for tonight are to watch soaryns factorio run

[paper cedar]

I will do it in a few hours i am just on the run

[rain herald]

on the run

rip

[paper cedar]

On the run from rush hour traffic? 😄

Anyhow... I can "live blog" the steps later, it's pretty trivial

it should be exactly as making a "normal" PR to neo with 3-4 extra steps

[high sonnet]

The link takes me to a page not found.... Where is the change log? Did they release anything??

[visual basin]

https://www.minecraft.net/en-us/article/minecraft-26-1-snapshot-4

[plucky hollow]

oh yeah the bot has not been updated to use the new article link format iirc
but my pins should always have working links

[bold forge]

now it is

[paper cedar]

Alright. Updating

This will be a bit of a stream of consciousness...

• Make new Git branch port/26.1-snapshot-4 from port/26.1
• Set minecraft_version=26.1-snapshot-4 in gradle.properties
• Run ./gradlew setup -Pupdating=true

Oh, NeoForm didn't actually publish

[plucky hollow]

Could not find net.neoforged:neoform:26.1-snapshot-4-1.
oh did neoform not update yet

[paper cedar]

Yep 😄

Checking https://github.com/neoforged/NeoForm/actions

Alrighty then, checking out NeoForm

• Set minecraftVersion = '26.1-snapshot-4' in settings.gradle
• Run createPatchWorkspaceForUpdate gradle task
• Reload Gradle Project in IntelliJ to make it pick up the workspace

Looks like this, pretty much the same thing as we do in NeoForge:

• Just fix compile issues until the gradle project compiles (in this case, reference the rejected .patch files since what needs to be patched is 99% the same stuff)

• Notice that createPatchWorkspaceForUpdate didn't actually clean the workspace first, so it contained classes deleted by Mojang, which then got turned into patches -> Annoying. (This is a bug with the NeoForm gradle setup)

[plucky hollow]

is the createPatchWorkspaceForUpdate failing expected?

[paper cedar]

Yes if it has rejected hunks

But it still creates the workspace and writes the rejects

If you reload the gradle project in IntelliJ after running that task (whether it fails or not), a new toplevel module should show up ("workspace"), which contains the MC code

I did copy paste over the rejected hunks, and got it to compile, so no new patches needed.

• Once done, run createPatches Gradle task (which does the same thing as genPatches on Neo does)
• Commit & Push
• Watch it check the commit on Github: https://github.com/neoforged/NeoForm/actions/runs/21183225940

80 compile errors on Eclipse, wat

Fucking Eclipse

• Run the testDataWithEclipseCompiler task locally to repro this (i just tested compile with gradle)
• Yep, 80 compile errors with ECJ....
• Give up and start Eclipse
• Import the NeoForm project
• Let it build... (yep 80+ errors)
• Slap explicit generics on a bunch of method calls until it compiles in Eclipse
• Run createPatches and testDataWithEclipseCompiler
• Commit & Push (https://github.com/neoforged/NeoForm/commit/95cc13869d72c95a4a4df58be07056bbaf883d6b)
• Watch it build and test again (https://github.com/neoforged/NeoForm/actions/runs/21183764980)

[plucky hollow]

why arnt all those generics being inherited like i would expect them to be?
is that just a eclipse thing?

[paper cedar]

Yeah

Sometimes it's the other way around where the Eclipse Compiler infers it, while javac does not

• Checks passed in GH
• Manually trigger the Publish Workflow (and check 'Create a Release') https://github.com/neoforged/NeoForm/actions/workflows/publish.yml
• Watch it build https://github.com/neoforged/NeoForm/actions/runs/21183926420
• Build passed, habemus NeoForm

[plucky hollow]

https://www.youtube.com/watch?v=Xhq6WsoUYbE

[paper cedar]

So, picking NeoForge back up where I left off.

• Make new Git branch port/26.1-snapshot-4 from port/26.1
• Set minecraft_version=26.1-snapshot-4 in gradle.properties
• Run generateAccessTransformers and applyAccessTransformer and fix until ATs are fixed (none broken in this snapshot) (i always forget to do this before the first setup so I get access changes in patches heh)
• Run ./gradlew setup -Pupdating=true
• Produces a bunch of rejects in the rejects folder
• Rename the rejects folder to rejects_26.1-snapshot-4
• Run genPatches
• Add renamed rejects folder to Git
• Commit & Push https://github.com/neoforged/NeoForge/commit/3ae5cea802d52426c1ecb1e7a2fc065e72ac673c

Now personal preference. I don't like working with .rej files, so I convert the .rejects back into .patches using a python script I linked here a long while ago (https://gist.github.com/shartte/2c175af29a14d61f0fc832f182900df9)

Which does this: https://github.com/neoforged/NeoForge/commit/7c488ce3b3133cf66579feb1292685bc88c86049

[plucky hollow]

and now its porting time i assume

[paper cedar]

Yep, work through rejects first

Reapply the patches by hand that it couldn't do automatically.

[wintry flower]

Make sure you're using Sync and not Copy

[paper cedar]

I personally delete projects/base/src since I always get confused when opening a file by name.

[wintry flower]

Maybe run :neoforge:setup then

[plucky hollow]

i comment out the project include in settings gradle tbh

[paper cedar]

Also possible 😄

[wintry flower]

I personally like to have base in case I need to look something up, and for the very tough updates it allows me to run the game with a debugger

[paper cedar]

Yes sometimes

[wintry flower]

We could have a Gradle property that disables it, then you could disable it globally

[paper cedar]

I do that once the rejects are back in

and I transition to fixing the compile errors

[wintry flower]

Sure yeah that makes sense

[paper cedar]

But I've reapplied a patch to the java file in base too many times 😄

hence I delete it during reject fixing

that's another personal preference thing

[wintry flower]

Did you check if neoform is using Copy instead of Sync?

[paper cedar]

Nope 😄

[wintry flower]

[paper cedar]

But okay okay, I'll go and chekc

It's using neither, actually

It's running DiffPatch file-by-file so it can report for each reject which patch failed

And even more importantly, it can report, which patches did not get applied at all

[wintry flower]

Isn't the rejects folder sufficient?

[paper cedar]

Doesnt help you in the github actions log

[wintry flower]

True

[paper cedar]

But yeah could be written differently, I suppose

BTW the reason to convert the rejects to .patch files is so IntelliJs diff view works:

I find that easier to read than the raw patch sometimes

[plucky hollow]

yeah side by side much easier

also what order are you doing rejects in? i started from bottom

[paper cedar]

I just randomly pick usually, this time bottom

But: it's so few rejects

this will be done in 10 min

This is also the place where you find some weird old patches 😄

our current patch removes the generator.run(), is that because it runs later?

Ah yes, it just needs it to add its providers, I suppose

[plucky hollow]

oh DataGenerator lost the rootOutputFolder field i need that for my datagen to function
i rely on getPackOutput(String)

it moved into the cached sub class

[paper cedar]

Huh, another weird one

private final Map<String, DataProvider> providersView = java.util.Collections.unmodifiableMap(this.providersToRun);

We patch that in, now i have to open an old dev env to see if that is even used

[plucky hollow]

getProvidersView method

[paper cedar]

ah!

[plucky hollow]

with the cache/uncache generators are we gonna expose some way for modders to define if their datagen is (un)cached?

assuming the uncache means it always generates, not skipping if cache exists

[paper cedar]

Hm

[visual basin]

is that different from doing this?

PathProvider pathProvider = event.getGenerator().getPackOutput().createPathProvider(packOutputTarget, folderName);

[paper cedar]

It seems to be unused in mojangs code

[plucky hollow]

cause i would like to use the uncache in CI so my local changes test always generates files and validates against that

[paper cedar]

Potentially used by an entrypoint we dont have

Uh... Are you checking in the .cache or what are you doing 😄

[plucky hollow]

something i could see neo doing for its ci too tbh

[west thorn]

Indeed
Some data now ends up in in-memory FS when generating for dev mode

[plucky hollow]

im thinking we add a --uncached arg to jopt or something?

[paper cedar]

Yeah IDK how that affects you in CI/CD Apex, we just dont check in the .cache folder, meaning CI should always regenerate as far as I understand

Well certainly possible but let's get it to work first 😄

[plucky hollow]

oh we dont?

some reason i thought we did
i mean im almost certain ive had times where ive ran data gen and nothing changed
but i delete the cache folder rerun and then files updated

[paper cedar]

Maybe we should check it in 😄

Okay yeah, LevelRenderer definitely the biggest reject, all the other ones were trivial

Where did the particle pass go 🤔

[plucky hollow]

flat out removed

according to snowman

[paper cedar]

Seems to have moved to particlestate#submit

[plucky hollow]

going by what lolo posted yeah, they are in the main pass now

[paper cedar]

• All rejects reapplied
• Rejects deleted
• Commit & Push (don't forget to add patches that are now "new") (https://github.com/neoforged/NeoForge/commit/e6fc24102e7b8cdeae4fff5d5c0d622b7704728a)

Now... well. hammer compile button, see what kaputt.

lol. IntelliJ kaputt.

[plucky hollow]

only 1 error that doesnt feel right

[paper cedar]

Yeah. Same thought here

I mean.... Let's try runClient 😄

Yeah uh... ok... starts

[plucky hollow]

noice

[paper cedar]

Well, let's see.... the usual cleanup crap

• run applyAllFormatting
• run check
• run runGametestServer
• run testProductionClient
• run testProductionServer

[visual basin]

snapshot-4.2.02091283920813-2137gmo not distribute

[plucky hollow]

also i like that tag "what do you expect"
like yeah it worked what did you expect crashes

[paper cedar]

🤔 everything so far passes

[steady heath]

https://github.com/neoforged/NeoForge/blob/port/26.1-snapshot-4/patches/net/minecraft/client/renderer/LevelRenderer.java.patch#L117 useless patch

[plucky hollow]

now push and watch ci pull some issue out of thin air

[paper cedar]

Still waiting for the prod server selftest

Ugh I thought I fixed this
java.nio.file.FileAlreadyExistsException: Z:\NeoForge\projects\neoforge\build\tmp\testProductionServer\server_self_test.txt

When running locally, it doesn't remove the self-test file first... failing when you run it repeatedly

Heh, I only fixed that for the client self-test

Ok passes too,

• Commit and Push (https://github.com/neoforged/NeoForge/commit/d35a626cc2222bd48a234c147dfb5a9da70b5139)
• Open PR (https://github.com/neoforged/NeoForge/pull/2936)

Should've opened the PR earlier, but whatever

[steady heath]

🥺

[paper cedar]

ah yes good catch

[plucky hollow]

block feature renderer patch is mostly all indented towards the bottom, is that intended?

[paper cedar]

Oh you mean one level indented too much for the replaced line

no

[plucky hollow]

its more than 1 line tho

im looking at the green whitespace

[paper cedar]

That's a change in context though

Also probably the reason it became a reject in the first place

If the whitespace change was from our edits, it would have to show up as a -/+ delta

[plucky hollow]

ah, yeah always found reading these double patches hard on github
also empty lines in datagenerator?

[paper cedar]

Same, I find it patches like that terrible to read on GH

Yeah those are probably an oversight too

and now I re-run setup so I get base to be able to actually fix that more easily 😄

Ah no, those are also context changes

If it was an actual edit it would have to be a line starting with + + or something like that

Basically if the diff shows you that the .patch file gained a line, but that line in the .patch file doesn't start with - or +, it's just surrounding, unchanged lines from vanilla

[plucky hollow]

oh didnt run datagen

[paper cedar]

Ah true

I always forget

[plucky hollow]

thats why we have ci

[paper cedar]

Done 😄

Yes!

Aight! Merged!

Anything else from 1.21.x we should cherry pick over before doing a release?

[plucky hollow]

unsure tbh

nice no code changes for my albeit simple mods

[paper cedar]

well -snapshot-3 is much harder hehe

[plucky hollow]

oh ofc i too forget datagen

[plucky hollow]

is vanilla rly missing these translations? or did we break something?

Missing subtitle translation{key='subtitles.entity.baby_horse.hurt', args=[]} for sound event: minecraft:entity.baby_horse.hurt
Missing subtitle translation{key='subtitles.entity.baby_horse.death', args=[]} for sound event: minecraft:entity.baby_horse.death
Missing subtitle translation{key='subtitles.entity.baby_horse.ambient', args=[]} for sound event: minecraft:entity.baby_horse.ambient
Missing subtitle translation{key='subtitles.entity.baby_horse.eat', args=[]} for sound event: minecraft:entity.baby_horse.eat
Missing subtitle translation{key='subtitles.entity.baby_horse.breathe', args=[]} for sound event: minecraft:entity.baby_horse.breathe
Missing subtitle translation{key='subtitles.entity.baby_horse.land', args=[]} for sound event: minecraft:entity.baby_horse.land
Missing subtitle translation{key='subtitles.entity.baby_horse.angry', args=[]} for sound event: minecraft:entity.baby_horse.angry

[visual basin]

translations and a few other asset types are not shipped with the jar but downloaded by mc separately

sometimes that fails and you get missing translations or sounds

[plucky hollow]

nope looks lke they are infact missing them

and yep launching vanilla double confirms that, so mayhaps hotfix in bound to add them

ah and its already reported nice
https://bugs.mojang.com/browse/MC/issues/MC-305862

[empty night]

MRW baby horses https://www.youtube.com/watch?v=bn3NNZyW0GQ

[sleek sinew]

they are missing. I spotted them on Xisuma's video

(oh nvm you confirmed it yourself)

[paper cedar]

I did port a bunch of the commits from 21.x

but the commit for adding caps to the bundle is more annoying

[inner creek]

unlikely, it doesn't hurt the game to be missing them, they can just fix the file in the next snapshot

[wintry flower]

Hehe yes

At least there's a test

[plucky hollow]

well lets see what peoples thoughts are on using the new uncached generator via a new --uncached arg
https://github.com/neoforged/NeoForge/pull/2939

[untold oyster]

@plucky hollow good news
after killing the runner and letting it sit for a few hours, then breathing it back to life, it's now running again

I'm not sure why it didn't fix itself when I first tried to do so some 12 hours ago, but ah well

it's running now

[plucky hollow]

and with that neo snowman now has nsapshot 4 yay
https://github.com/neoforged/Snowman/commit/a680b7c5a7fb3af094e1df0378a2f8098dd066d6

[paper cedar]

@wintry flower do comfortably do the bundle stuff, we need some new utils in ItemResource, IMHO

I'll do those in a separate commit ahead

Since you are our utility connoisseur

[wintry flower]

You want me to do it? ^^

[paper cedar]

What we also need is a isSameItemSameComponents for ItemStackTemplate / ItemStack

[round spear]

is the super of both of those close enough that it can just be changed to use that? iirc you said they share a super class/interface

[paper cedar]

You can't iterate the components on the super

@wintry flower https://github.com/neoforged/NeoForge/pull/2940/changes

[robust pike]

Take your primer: https://github.com/neoforged/.github/pull/39/commits/30c52760603324b9c389c73df46d51c32f330c2a

Split of translucent and solid feature rendering, and some activities / brain changes

[wintry flower]

OK I left a small comment

do you want me to take care of tommy's PR or what?

[paper cedar]

Hm?

WDYM?

The bundle cap one?

[wintry flower]

yeah

[paper cedar]

Nah I already ported that on top of those helpers

[wintry flower]

ah I see

[paper cedar]

The bundle content is now a List<ItemStackTemplate>

[wintry flower]

is guideme already available for snapshot 4?

[paper cedar]

I could have ported it by always converting every element to an ItemStack before matching it...

[paper cedar]

yeah

[wintry flower]

sweet 🙂

[paper cedar]

Ah wait, no

Since there's no NF for snapshot-4 yet

I want to do tommys PR, then release

[wintry flower]

ok well if you're lazy just merge https://github.com/neoforged/NeoForge/pull/2940

otherwise add the count-aware match as well 😄

[paper cedar]

I am on that

Uh while I am looking at that

isSameItem too?

How does that behave for empty stacks, btw.?

Is ItemStack.EMPTY.is(Items.AIR) true?

[wintry flower]

yes

    @Override
    public Holder<Item> typeHolder() {
        return (Holder<Item>)(this.isEmpty() ? Items.AIR.builtInRegistryHolder() : this.item);
    }

[paper cedar]

Ah hah

But since ItemSTackTemplate cant be made for empty stacks

because null represents that heh

That's a bit awkward

[wintry flower]

well just take a nullable template, no?

[paper cedar]

public static boolean isSameItem(ItemStack a, @Nullable ItemStackTemplate b) {
    return b == null ? a.isEmpty() : a.is(b.item());
}

[wintry flower]

we could also squeeze https://github.com/neoforged/NeoForge/pull/2916 into the release, I am updating it right now

[paper cedar]

Ok

I don't see the method that also compares the count hehe

[wintry flower]

ItemStack.matches?

[paper cedar]

ah yes 😄

[wintry flower]

26.1.0.0-alpha.26.1-snapshot-4.20260121.215839 is definitely not a correct version

what should we use instead?

[paper cedar]

Those look fine for you? https://github.com/neoforged/NeoForge/pull/2940/changes/89bf9d19eebbd16c1068ead1d2a8db97dbcf4798

[wintry flower]

26.1.0.0-alpha.??+snapshot-4.20260121.215839 would be good