1.21.2 Snapshots

[tidal siren]

iirc you couldn't set the default gamemode to hardcore or something
You'd needed to place and run some commandblocks

[tropic anchor]

the recipe only copies the components of the input item

Also apart from the input item the transmute recipe type only allows for 1 other item

[haughty raven]

interesting block properties got a small refactor
DirectionProperty is gone and now EnumProperty is more configurable when its initially created
to specify which enum consts it uses
all property classes look to have been finalized and ctors made private

[drifting ice]

Well that’s boring

Useful but boring since I would think copying data components would be a given for stack based recipes

[cosmic acorn]

thankfully that won't affect my packing tape logic

I mean in the sense that my logic was already not using DirectionProperty :P

[haughty raven]

https://github.com/neoforged/Snowman/commit/01a71a210b6ea8e224a2bc5752c9c68f0008e45e

[tidal siren]

I have been

[spiral tapir]

fabricord people said that shulker boxes now have an item tag? so that should be removed from the c tags

[round perch]

They do, yeah

[tidal siren]

Deprecated for a version (along with including them in the vanilla tag)?

Can't remember how we've handled tag removals like that in the past

[round perch]

@lucid totem They did exactly what I suggested last week for the index lookup in EnumProperty \o/

[lucid totem]

Neat

[timber jolt]

https://tenor.com/view/mike-wazowski-watchingyou-gif-5352035

[blazing steppe]

For c tags we need to discuss with fabric

[bronze crescent]

Dispensers be doin' stuff o.o

[round perch]

They just moved the minecart dispense behaviour to a proper class from an anonymous one in the minecart item

[tidal siren]

I don’t be doing anything

[bronze crescent]

Oh neat, Player#canDropItems is a thing now

Seems prime for a small patch there

Hey Mojang, if you're listening, a playersMayDropItems gamerule would be kinda nice

[round perch]

It's overriden in both server and local player for drop spam throttling

[bronze crescent]

So you can have a survival player in a map, but they can only do inventory management at chests/etc

[cosmic acorn]

if, on top of a no drop items gamerule, we also got a rule to limit the available inventory slots, we could make one-block-at-a-time with vanilla

[bronze crescent]

Oh lawd

You may only have armor, main hand, and offhand

Get creative.

Subtle:

DirectionProperty is gone, replaced by EnumProperty<Direction>

[terse anvil]

Resident Evil moment

[drifting ice]

And done with the primer: https://github.com/ChampionAsh5357/neoforged-github/commit/984b5c84829f4f1a1e8f2dc0413bfd9476740ea4

[hallow basin]

player drop speed attribute, set to 0 to disable

[spiral tapir]

mojang

wonder why consume effects aren't data components themselves

[drifting ice]

They are? Well, the holder of the consume effects is

[spiral tapir]

yeah, I'd have just expected them to be similar to the enchantment effects ig. though, I suppose those are doubly split into components and separate non-component registries as well. just strange how it's shaping up rn

[kind fractal]

probably to have consume effects applied in a visible, reliable order; totem of undying's death_effects clear all status effects and then give a couple of effects afterwards

[spiral tapir]

I mean, doesn't stop them from making a DataComponentList or DataComponentLinkedMap

the ConsumeEffect.Type is just the same stuff as a DataComponentType

minus the cache

[inland vault]

New version detected: 24w39a.

New version detected: 24w39a.

[blazing steppe]

<@&1067092163520909374>
https://www.minecraft.net/en-us/article/minecraft-snapshot-24w39a

[inland vault]

New version detected: 24w39a.

[lunar dagger]

WHAT

[bronze crescent]

here we goooooooo

[prisma pawn]

[red jungle]

Bundles are no longer experimental

[prisma pawn]

a snapshot so good they released it three times

[undone vortex]

Yo

What's the link

[red jungle]

BoatType enum might be gone?

[jaunty zodiac]

https://www.minecraft.net/en-us/article/minecraft-snapshot-24w39a

[true grotto]

I would have thought that map_invisibility_equipment would've been a data component not an item tag.

[bronze crescent]

Where were you when Boat Split

[round perch]

Looks like it, yeah, they are all individual entity types now

[hoary cipher]

No more annoying type enum

[round perch]

One less enum to make extensible

[lunar dagger]

dammit, Snowman is regenning the commits again!

[blazing steppe]

hmm smol one

[lunar dagger]

i hate it

[bronze crescent]

Oh cool, container locks are a predicate now

Not that anyone ever used that /s

[blazing steppe]

MATYYYYY

[prisma pawn]

finally, caves and cliffs is over

[lunar dagger]

i screm at n8n

why is it not debouncing

[hoary cipher]

I've had an issue open of someone wanting me to make the rafts in Just A Raft Mod extendable so they could register more raft types for modded trees using KubeJS

[blazing steppe]

because maty didn't configure it to

[red jungle]

Why would it not?

The custom runner should have the cache?

[lunar dagger]

note that it isn't decompiling all the versions again; the cache still exists for those

[red jungle]

Oh what

Just committing takes forever?

[lunar dagger]

but because the repo got into a bad state due to the two interrupted workflows -- because the n8n workflow executed thrice in succession -- it's recreating all the commits

[bronze crescent]

Automated automation is hard, apparently

[lunar dagger]

it's an idiosyncrasy of Snowblower; it pulls down the entire repo, and force-pushes the existing commits up in batches of 10
but if it gets interrupted while doing that for the old, previously-made commits, then the GH repo contains only the first few old commits

and it has to recreate all the old commits

[inland vault]

Minecraft Snapshot 24w39a

Happy Wednesday miners! With this snapshot we're now bundling Bundles into the game for real - Bundles are no longer experimental!Happy mining!New FeaturesBundles are no longer experimentalChangesUpdated slot highlight in the UI so Item texture has better ...

Sep 25, 2024, 1:58 PM

[lunar dagger]

n8n thonks for the workflow: put the 'success' part of the existing workflow into a new workflow, into which is passed all the info (modified time, versions)
and set that new workflow to store all successful executions

[cosmic acorn]

The Flattening, part N+1

[dapper pendant]

extensibleEnumCount--

[tidal siren]

I in 211 of 281

[tidal siren]

https://github.com/neoforged/Snowman/commit/bfaf938b44b3e6e2378203f7cd0aff817fe4b3fa

[spiral tapir]

Finally, boats are good

[red jungle]

a lot of server-exclusive methods were moved from Level to ServerLevel apparently

that seems to explain a lot of the diff

[dapper pendant]

Yeah, because then a ServerLevel is passed as context to a lot of Entity methods so that those methods don't have to (ServerLevel)level() everywhere

[bronze crescent]

Screen#showsActiveEffects is a nice little override

Looks like a ton of recipe book handling got turned into AbstractRecipeBookScreen, too - simplifies a lot

LocalPlayer#hurt is gone, RemotePlayer#hurt is now #hurtClient -- so we got some MP changes there too

[heavy raven]

They remove Herobrine again?

[dapper pendant]

No that's only in full release changelogs

[heavy raven]

Ahhh right 👍

[bronze crescent]

Herobrine now knows how to use non-oak boats now, though

So be warned.

ByteBufCodecs now has LONG 🎉

[heavy raven]

Good

[bronze crescent]

Looks like that's because the clientbound time packet now encodes the game, day, and tick times

(It's a record now)

New interface, ServerEntityGetter, extending the existing EntityGetter -- so yeah I'd agree with Tech, Mojang is shuffling around client/server logic a decent bit here

Notably, ClientLevelData no longer seems to have a copy of the game rules

[blazing steppe]

tech how far is the split?

[scarlet pawn]

I am supposed to review it still

[red jungle]

gradle-wise or client/common-wise?

[blazing steppe]

Client common, the thing we are waiting on to start kits

[red jungle]

let's not wait for that

it will make rebasing stuff painful, so I'd rather delay it as much as possible

[round perch]

Sounds reasonable. I'd be down to waste a few hours tonight, hammering out rejects, if we decide to start with kits

[red jungle]

let's try it

how tf did it run > Task :neoforge:idePostSync lol

(on GH actions)

[round perch]

lol

[red jungle]

we should try running the workflow with the buildsrc plugin to see if it's faster 😅

(and if -Pupdating works)

anyway; off you go 😄

[round perch]

👌

[red jungle]

202 rejects

[round perch]

Time to have some fun 😄

[red jungle]

fuck I have two hackmd accounts -_-

one with github and one in my password manager

https://hackmd.io/@neoforged/Sk7XYpW0R

might be useful, might not be

let me already claim custom ingredients 😄

[round perch]

I'm gonna claim the client package 😅

[blazing steppe]

I have not looked but I'm willing to do block stuff

[round perch]

Block is something I'm likely gonna touch as well because it has some interactions with the rendering rejects I'm fixing

[red jungle]

remember to push sometimes 😄

[red jungle]

ingredients can't be empty??

[kind fractal]

isn't it the fields that are optional?

[red jungle]

yes that's how it works now

pretty crazy 😄

[round perch]

Currently digging through the weeds of baked model rendering and all the fabulous/non-fabulous render type differentiation is gone \o/

[red jungle]

🤨

[round perch]

All the methods for baked model rendering that deals with DefaultVertexFormat.NEW_ENTITY render types (mainly items and falling block entity) previously threw around a boolean flag everywhere. That's entirely gone

One example are these two methods: https://github.com/neoforged/NeoForge/blob/1.21.x/patches/net/minecraft/client/renderer/ItemBlockRenderTypes.java.patch#L41-L54

[red jungle]

ahhh

I see

that's good

[round perch]

Yup

[red jungle]

I think we'll drop the Ingredient MapCodec

kinda sucks for SizedIngredient though

[round perch]

The move of LightTexture computation to a shader might fuck over some people as IDimensionSpecialEffectsExtension#adjustLightmapColors() won't work anymore (at allowed per-pixel light color adjustments)

The "entity render state" stuff also has some interesting consequences

[blazing steppe]

Guys don't forget to do regular commits and push them

Xfact for later, we normally do one (or more) commits per reject file

[round perch]

I can't remember that we ever did one commit per reject. It also just doesn't make sense with how intertwined these rejects often are

[blazing steppe]

Since i joined the porting team we mostly did 1 commit per reject file

[red jungle]

that doesn't make sense

we commit stuff in batches of relevant stuff

looks like some people were splitting stuff by patch; others not

[round perch]

Probably the greatest typo in this codebase

[red jungle]

I turned myself into a partial tick, Morty!

[haughty raven]

oh are we porting kits now
guess i should go clone it and help out

[red jungle]

that's right

[red jungle]

I think the

[ "minecraft:planks", { "type": "modid:whatever", "data1": 42 } ]

syntax won't be implemented

instead it will be

{
  "type": "neoforge:compound",
  "children": [ "minecraft:planks", { "type": "modid:whatever", "data1": 42 } ]
}

[round perch]

Seems reasonable. The verbosity doesn't hurt IMO

[red jungle]

yes, it's acceptable

hmmm

I just realized that { "type": ... } will conflict with custom holdersets

[blazing steppe]

oh because ingredients now use holdersets

[red jungle]

yes

[round perch]

Yeah 😄

[blazing steppe]

[red jungle]

the patches are still simpler because all the Value crap is gone

(i.e. the complexity level went from 8 to 6 out of 10 or so )

[blazing steppe]

nice

[red jungle]

well 6/10 is still a lot

unfortunately we cannot remove the custom ingredients, because NBT-aware matching is important

[blazing steppe]

but we need a new name for the ingredient type field

[red jungle]

that is one option, yes

[blazing steppe]

imo it's the easiest option

[red jungle]

yes

let's do ingredient_type

damn the NonNullList<Ingredient> getIngredients() getter is gone from Recipe

we'll have to patch some shit like that back in 😒

(otherwise mods like AE2 or JEI have no way of guessing possible inputs...)

we'll see later

[vivid ore]

I feel like it shouldn't be necessary to patch back in -- after all, what does the vanilla recipe book use now? Shouldn't JEI and company be able to do whatever it does?

[spiral tapir]

PlacementInfo?

[vivid ore]

(I'm also generally against patching methods onto classes that modders would be forced to implement in general. If a modder implements the vanilla contract for a class, it should be enough)

[red jungle]

complex recipes simply aren't supported by the guide book

[red jungle]

yes; that is unfortunately hardcoded to Holder<Item> (at the moment, at least), and custom recipes just return PlacementInfo.NOT_PLACEABLE

[dull heron]

huh? how do shaped recipes with empty slots work

[vivid ore]

Shaped recipes are a "shape" defined with placeholders and then a map of those placeholders to ingredients

There's no ingredients for spots that don't have anything in them

[spiral tapir]

it is a little annoying that you don't have context to whether something is a tag ingredient or a regular ingredient, etc, anymore

[vivid ore]

Seems like the proper approach here is to see about extending PlacementInfo instead of patching in a method that would mean that a Recipe implemented against the vanilla contract is invalid in neo-land...

So I suppose, how hard does making PlacementInfo provide more functionality look?

...Though actually, wait -- can you give me an example of a recipe that is placeable but where using Holder<Item> isn't enough to do so? I suppose stuff with custom NBT ingredients or whatever, right... hmm

If that method is patched back in, there should be some very clear docs noting that mods cannot rely on recipes necessarily implementing it (and maybe directing them to PlacementInfo stuff as a fallback?)

[haughty raven]

how do we handle when vanilla introduces a new system
for something we previously patched in?

vanilla has a new conversion thing in mob Mob#convertTo
to convert from 1 mob type to another
we patch in our conversion events everywhere
vanilla tries to convert an entity

i feel these events should now be moved from all the various entity classes
to now live inside of the convertTo utility

[round perch]

Probably, yeah

[haughty raven]

would maybe make sense to work on that tho after the initial porting(?)
(apply rejects like normal for now, move them to mob convert later?)

[round perch]

Yeah

[haughty raven]

ahh we pushed at same time xfact

[dull heron]

What's new about Mob#convertTo

[haughty raven]

wait thats not new? i thought it was added in one of the recent snapshots

but anyways i think rather than patching in conversion events everywhere
throw them in the conversion api vanilla has
less patches, same functionality

[dull heron]

The signature is different

but the method isn't new

[sand raven]

I am veeery certain we have the patches because of edge cases convertTo misses...

it's why I did several of the patches

[dull heron]

Well there's a new object

ConversionParams

which is backed by an enum ConversionType

[sand raven]

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

[dull heron]

https://github.com/neoforged/Kits/blob/24w39a/projects/neoforge/src/main/java/net/minecraft/world/entity/ConversionType.java

[sand raven]

unless vanilla changed to actually call convertTo for these cases, the patch stays

[haughty raven]

vanilla is using it everywhere now

[dull heron]

It doesn't really help with .Pre, I think, because the ConversionType isn't invoked until the new entity has already been created

[haughty raven]

events patched in are callbed right before and after vanillas convertTo

even slimes are using it for splitting on death

[dull heron]

and Mob#convertTo is still virtual, so we don't have a good entrypoint to call pre

It's still possible we can improve the API with vanilla's new changes

I can have a look once snapshot time is over™️

[round perch]

I'm gonna call it quits for tonight, I've stared at these stupid patches long enough. Some notes:

• RenderNameTagEvent will need to be split into "allow rendering" and "do custom rendering" due to the "entity render state" changes
• RegistryAwareItemModelShaper should not be needed anymore as the vanilla ItemModelShaper now stores a Map<ResourceLocation, BakedModel> to support the "item model" data component
• RenderPlayerEvent can no longer subclass PlayerEvent due to the "entity render state" changes (satisfies #1266)
• LevelRenderer#outlineEffectRequested needs to be moved to a FrameGraphSetupEvent due to the frame graph system
• Separately rendering non-translucent particles may not be viable anymore due to the frame graph system
• AbstractFurnaceBlockEntity burn time handling patches need handling of FuelValues
• ChunkDataEvent.Load/Save need to be reconsidered (chunk object and NBT data are not simultaneously accessible anymore)
  The only one of those that I've implemented is the RenderPlayerEvent change

A side-note on the last point: due to ChunkSerializer not existing anymore (saving happens via SerializableChunkData now), the associated patch did not even produce a reject, which seems very error-prone to me

[lunar dagger]

hmm, that last one might want @glass perch's attention

a patch not being considered a reject when the original file that the patch modifies doesn't exist

[glass perch]

Hmm, i sware there were unit tests for that ._.

Will poke DiffPatch later tonight

[spiral tapir]

rip https://github.com/neoforged/NeoForge/pull/1304

[tropic anchor]

I think DiffPatch doesn't add the .rej file extension if the file that is being patched was removed and the workflow for starting a kits branch checks for .rej

[lunar dagger]

i'll have my top man for the job on the case

that man being covers

[red jungle]

it's just a matter of running the steps locally

[red jungle]

fun fact: we have the same problem over the network

I suppose we'll use a size of -1000 for custom ingredients

[vapid drum]

You could use {type:"...", value:{...}}

It is even more verbose though

[red jungle]

over the network I am using a prefix of -1000 for now

for custom ingredients it is "ingredient_type": ...

[vapid drum]

Has Neo considered prefixing custom keys with its own namespace like Fabric does?

[ancient frost]

we have in other contexts

[spiral tapir]

Load conditions, for example

[red jungle]

yeah, we don't do it consistently atm

I'll do fluid ingredients once everything compiles and runs; for now I think they should still compile and run using the old code

recipes are now deserialized in the prepare phase of the reload listener

this is likely problematic for resource conditions

looks fine for tags though

yeah it's fine, tags are now loaded before the reload listeners are even instantiated

[vivid ore]

If we're chaning it anyways, could we prefix it with neoforge:?

[red jungle]

alright let's do it

@round perch I read that as All Rejects

😄

[round perch]

[red jungle]

currently looking at IContext

[round perch]

I had a quick look at that last night and noped out 😄

[red jungle]

hehe 😄

it got changed quite a bit; now vanilla has a thing called PendingTags

which contains a registry lookup that you can use to get tags

however it looks like the contents of the tags are not available yet (imagine the bugs that people will run into!) 😄

[round perch]

Oof, yeah, that's going to be interesting

[red jungle]

oh apparently the tags are just updated in the HolderLookup.Provider already 🤨

so we might be able to remove IContext entirely

I think we can remove it, but I'll keep it around for now since it's reasonably easy to adjust

[round perch]

Sounds good

[red jungle]

can't wait for mojang to start using virtual threads so we can stop with all the future soup

[round perch]

Hehehe

[red jungle]

do you also have this weird compile error?

(this is in base)

[round perch]

That class doesn't exist in my dev env

[red jungle]

right

we've had this problem before

the magic command is git clean -dxf -- projects/base/src projects/neoforge/src

(to remove all untracked files)

be careful 😄

[round perch]

My magic is to just nuke the local folder

[red jungle]

lame 😄

[round perch]

Ah, so that's where that annoying command source error came from

[red jungle]

yup 😄

[round perch]

Not sure this is the best solution to deal with the mob conversion changes with respect to the mob splitting event, but it should do the trick: https://github.com/neoforged/Kits/commit/89cf439f9de26521283dd7e8636f06fd16feaf42#diff-40e51d77c70913b0520424a1c1d6ef6667bd980da035b914fca7985e9dbdb75dR217-R229

[red jungle]

// Neo: Shear logic is handled by IShearable

scary

I hope it's correct 😄

[round perch]

IShearable just defers back to Shearable for the relevant vanilla entities

[red jungle]

https://github.com/neoforged/actions/pull/5

btw

limited testing suggests it should work 😄

[red jungle]

oops, accidentally wiped everything in the directory that is not in Git 😅

[round perch]

We'll have to check how this interacts with ReloadableServerRegistries' use of LootDataType#conditionalCodec() when calling scanDirectory() since that would technically create a conditional conditional codec: https://github.com/neoforged/Kits/commit/6f27c2c51c94b67c671a4cc92f5092b11f1fb81b#diff-d6014723b910eeacb32858ad287febb5aa70a3013236c3bbfce18342ef9788eaR52

[red jungle]

that's not good 😄

https://github.com/neoforged/Kits/commit/f5e39dd9c4ea2798ffbbd86b2f4d98e55bd259a6

they're back

[round perch]

👌

I'll go through those once I'm done with projectile nonsense

[red jungle]

you're a machine, I don't have enough motivation for all this nonsense 😄

[round perch]

😅

[red jungle]

I wonder if we should have started the port earlier

[round perch]

Not sure to be honest, yesterday's snapshot did break quite a few things with the server level stuff in entities, so it might have actually been the perfect time

[red jungle]

the later we start, the less total effort it is

but it is more effort to get it to compile

[round perch]

Definitely

[red jungle]

what 😄

[ancient frost]

did mojang change Registry#get from nullable to optional

[sand raven]

Looong =

[ancient frost]

thanks I hate it

brain wants to stab me for using single equals sign for comparison

[round perch]

Due to Registry now extending HolderLookup.RegistryLookup and therefor HolderGetter, the value lookup is done with Registry#getValue() now and Registry#get() now does what Registry#getHolder() did before

[red jungle]

Hmmmm ok

[round perch]

Hmm, the effect curing via ConsumeEffect is a bit annoying. To do this properly with the EffectCures we'd need to register a custom ConsumeEffect.Type but that would break vanilla compat. Patching the two ConsumeEffects related to this is also not viable because the one used by milk is called ClearAllStatusEffectsConsumeEffect and the one used by honey bottles (RemoveStatusEffectsConsumeEffect) takes a HolderSet<MobEffect> which we can't populate from an EffectCure.
Mods will probably just have to use ModifyDefaultComponentsEvent to replace the specific ConsumeEffect held by the Consumable component on the milk bucket and honey bottle (with custom holdersets for removal) to add or remove their effects from the cleared ones

[red jungle]

somewhat related, but are we going to add IItemExtension methods for all these component queries? or should people just mutate the component accordingly whenever they change some other NBT?

[round perch]

Hmm, good question. I'd say leave it as-is for now with no extension methods and then expand on it when someone comes up with a good reason for an extension method

[red jungle]

yeah of course; but long-term it's not a viable solution to add an extension method for every single thing

[round perch]

Yeah, somewhat keeping these extensions to a minimum is definitely necessary

On a completely different note: the model baking changes are very worthy. We should probably look into nuking UnbakedGeometryHelper.bake() and instead patch BlockModel#bake() directly. Our ElementsModel in particular feels pretty pointless over SimpleBakedModel, the only difference is that the latter doesn't get the root transform treatment (which is slightly broken anyway at the moment)

[red jungle]

wtf is that ElementsModel

[lucid totem]

I approve of this idea

[round perch]

A SimpleUnbakedGeometry that produces a SimpleBakedModel with the only difference to the vanilla construction of the latter being that the former applies root transforms

To me it looks like one of those "avoid larger patches at all costs" kind of thing from the past

[sand raven]

#566276937035546624 message

[red jungle]

🤷

[round perch]

There is no world where this is actually the case...
What was the build.gradle line to change the max errors again?

[haughty raven]

-Xmaxerrs <count>

[round perch]

Perfect, thank you, just need to find the correct place in Neo's build script 😅

Yeah, that's more realistic

[haughty raven]

where did you place it, i tried adding it yesterday but couldnt get past the spread players error

[red jungle]

lol I had the same

[round perch]

In the allprojects block in the top-level build.gradle

[red jungle]

if we were running with idea it would be nicer

[round perch]

I'm gonna try to get rid of most of them in one run, shouldn't be too bad all things considered

[round perch]

That took ever so slightly longer than I anticipated, but I have a main menu and a test world. The loading overlay will need some fixes as it currently produces strobe lights

My last three commits make it compile and run but I have not handled all rejects yet. I'd like some additional looks over these three commits in particular:

• Untangling model loading (removes ElementsModel and UnbakedGeometryHelper#bake()): https://github.com/neoforged/Kits/commit/ce9cfbee2059f4fb8ffda173de27b8b56b391ca1
• Making it actually compile and run: https://github.com/neoforged/Kits/commit/3d2e48397dae962ba64a2a1f5d4451de4233ad5e
• Fixing interactions between tag binding and registry freezing after snapshot application (on this one I'm really not sure it's the best solution): https://github.com/neoforged/Kits/commit/aa3da9c8a238776ffbf75b61ca297a33c637ced2
  I've left quite a few TODOs in the code which I'll have to go over tomorrow

[haughty raven]

⚠️ flashing image warning
is this caused by all the render changes or is my graphics dieing?

[round perch]

That's a bug in the overlay. I'm not quite sure yet why it happens though

[haughty raven]

ah ok so not my graphics dieing xD

[round perch]

Yup. This issue is what I was referring to with strobe lights in my comment above 😄

[haughty raven]

oh you mentioned it already.. i didnt read xD just saw the "make it compile" commit and clicked run xD

[round perch]

[haughty raven]

also create test world, is that a vanilla thing?

[round perch]

Yes, that was added in a recent snapshot

[haughty raven]

oh nice

i assume its enabled cause in dev is true

[round perch]

Yup

[haughty raven]

gonna have to look and see how that works see if it sets the normal things i usually set

[round perch]

I'm gonna go get some sleep. I would appreciate comments on the three commits I linked above, there is very likely some stuff that could be done cleaner

[timber jolt]

what did the test world do again?

[lunar dagger]

iirc, a superflat 'redstone ready' preset world with daylight cycle, mob spawning, and weather cycle gamerules disabled

[red jungle]

That's epic

[timber jolt]

right, that's good

[ripe drum]

is that button behind the sharedconstants flag? that's wonderful

imagine the button changing to load test world if there's only one world in the folder and that's a test world

[scarlet pawn]

You should instead pass the cli flag to auto join the world on start

[ripe drum]

that's too much work

[round perch]

I'm currently looking at the remaining minecart rejects and I'm considering ripping out the entire minecart extensions. A lot of them make no sense with the experimental minecart behaviour, some are replaced one-to-one by new vanilla methods and a lot of the targets of the patches are moved to the MinecartBehaviours. We can always add back hooks that make sense for the new minecart implementation.
Any objections?

[lucid totem]

I'd be in favor of ripping them out to get the thing to compile as long as we revisit prior to the release

[round perch]

Well, it does already compile and run 😄

[lucid totem]

Oh right true

[round perch]

But we do need to get rid of the remaining rejects, so it's either painfully reimplementing them or ripping this out entirely

[red jungle]

We can rip it out, it's a super old API

[ancient frost]

yeet 🛒

[red jungle]

Maybe do it in a separate commit so we can always go digging back for it

[round perch]

Yeah, I'll make a dedicated commit for it

[round perch]

@red jungle shouldn't this be !ingredient.isCustom(): https://github.com/neoforged/Kits/blob/406eaf5ff4c52dbce594c42982e4e92bb96a93d5/src/main/java/net/neoforged/neoforge/common/crafting/IngredientCodecs.java#L56?
With the isSimple() check it currently also catches custom ingredients that wrap vanilla ingredients or other simple custom ingredients and therefor blows up on client sync since it tries to sync the empty placeholder HolderSet<Item> used by Ingredients holding a custom ingredient

[round perch]

I'm experimenting with the furnace fuel burn time stuff right now and my current approach is to build the datamap from the vanilla FuelValues during datagen (the generated file is unchanged) and then take over the runtime creation of FuelValues to instead populate it from the datamap. On the server I can do this directly in the two places where vanilla builds it (MinecraftServer constructor and MinecraftServer#reloadResources()). On the client I can't do that because datamaps aren't synced yet in the ClientPacketListener constructor, ClientPacketListener#handleUpdateTags() is only called for the tag sync after /reload but not for the initial sync since that happens during config phase and DataMapsUpdatedEvent only fires on the client for dedicated server connections. My current solution on the client is to hijack ClientPacketListener#handleUpdateRecipes() since that is called for both initial connection and reload.
Can anyone see any flaws in this idea or think of a better one?

[red jungle]

The check is correct but it needs to write the items manually

The intention is to send the items directly if the ingredient is simple

[round perch]

So basically change the vanilla stream codec to use Ingredient#items() instead of accessing the values field directly?

[red jungle]

Yes

Make sure it matches what the holderset codec expectsbof course

[round perch]

Yeah, that's going to be fun because items() returns a List<Holder<Item>> 😅

Though, I guess for the sync a direct holder set should be fine, right? I can make one of those from a List<Holder<Item>>

[red jungle]

Ah yes of course

[round perch]

This does the trick in my testing: https://github.com/neoforged/Kits/commit/826c1c20d7b2a2c6397dfd0f9aa269704394ca9a. I opted to use a little helper to preserve the ability of vanilla ingredients to sync a tag key instead of the resolved entry list

[red jungle]

They sync the tag key? I didn't know that

[round perch]

The HolderSet can be a HolderSet.Named which is synced as a tag key instead of the resolved entry list

[red jungle]

That's really cool

I wonder if we should always sync custom ingredients via their type then

To optimize the network footprint

[ancient frost]

vanilla clients go brrrrr

[red jungle]

Fun fact: we could make it depend on the connection type

That's a great idea actually

[round perch]

I was about to say 😄

[red jungle]

Something for tomorrow 😛

(unless you do it xfact)

[round perch]

Sure, I can look into it in a moment

How would you prefer to deal with complex custom ingredients and vanilla clients? Try a best-effort vanilla sync or disconnect?

[lucid totem]

Wait they finally fixed this?

In the past the client always seemed to get a massive list of items for tag ingredients

[ancient frost]

"always seemed"? that's literally how it worked

[round perch]

Yes

Tags are synced before recipes since 1.20.5, so it was finally possible to solve this

[lucid totem]

Cool

I only need to do hacks for 1.20 & 1.21 then

I was actually just looking into the problem yesterday

[round perch]

Implemented this as described, if someone has a better idea, feel free to change it: https://github.com/neoforged/Kits/commit/29741afee2719416c123ad9932b110dac6d65309

[round perch]

Here ya go: https://github.com/neoforged/Kits/commit/11fe5153ffd872d0c9137c83e6ed9a9a6c23469b. I've opted to have it sync custom ingredients on a best-effort basis to vanilla clients for now, though, if desired, we can always add back a check for simple ingredients and throw an exception when trying to sync a complex custom ingredient to a vanilla client

[round perch]

It's late as hell again, but we are now down to 11 rejects and 6 patches with lost targets and all of them have a comment outlining what might be done about them

[red jungle]

Looks good!

[red jungle]

You're crazy 😅

[ripe drum]

disclaimer: i haven't looked at new vanilla code
is there a reason we don't just ignore the furnacevalues and have the method return the data map

[lucid totem]

@round perch I've restored the two-pass particle rendering, it's not that bad: https://github.com/neoforged/Kits/commit/9c530732e9d0dab5c8083c01cf1f10a2ed36bd8f

[round perch]

👌

[lucid totem]

Do we know anything about what's causing the strobe effect during the loading overlay yet?

[round perch]

Nope, no idea yet

[red jungle]

did someone fix the 3 datagen files that were changing?

(c:boats tag, something with the piglin predicate, and the fuel data map)

[lucid totem]

c:boats still shows an error during world load so probably not

[round perch]

The fuel datamap doesn't change anymore since last night

[spiral tapir]

something something shulker box item tags

vanilla's added it now, so the c tag seems redundant

but that PR should probably be coordinated with fabric people

[round perch]

The data we load from the datamap fits neatly into the vanilla system, so it seems like a good idea to me to embrace the vanilla system that is almost guaranteed to be data-driven itself soon

[red jungle]

yeah I suggest we just fix the existing tags

and let TG have fun with changes 😄

ok I'll look at the other files then

[round perch]

What do we want to do about all the elytra hooks? Flying is now controlled by the GLIDER data component

[red jungle]

DataComponentType<Unit> GLIDER

😒

ok so it's a combo of the glider component and the equippable component

[round perch]

Mhm

[red jungle]

it just does a hurtAndBreak on the component while gliding

this kinda sucks

in MI I have a diesel jetpack that acts as an elytra

it doesn't take damage so that's pretty easy

for my use case at least, we can remove the hook 😄

[round perch]

How does it render? Is it an armor piece that uses the armor model hooks or is it a dedicated layer?

[red jungle]

it's really ugly atm, it just uses the vanilla armor model with a custom texture

[round perch]

Ah, makes sense.

The armor model hooks also got a significant hit since they entirely lost their entity context. Some of them still get the entity render state and some currently get nothing

[red jungle]

there is now #minecraft:boat so I'll leave only that in #c:boats (along with #forge:boats, which will probably get cleaned up at some point)

I love mojang's consistency btw

plural names? never heard of that

[round perch]

lol

[red jungle]

the advancement replacement fails because of the new PIGLIN_SAFE_ARMOR tag, it's just a matter of updating the provider

[lucid totem]

The overlay issue makes me think somebody's math is lerping with only 0 and 1 instead of the actual value

I generated patches to look at our diff vs vanilla but not seeing anything obvious yet

[red jungle]

the flashing?

[lucid totem]

Wait we completely replace the loading overlay

[round perch]

Yes

[lucid totem]

And do 5000 manual OpenGL functions

Maybe I should clean that up

[round perch]

Yeah, that was my thought as well 😄

[sand raven]

Vanilla tag is only passenger boats. Not all boats

So prob just c:boats/passenger (for clarity), c:boats/chest, and then have those two tags plus vanilla tag in c:boats

[red jungle]

really

[sand raven]

It’s for the boat with a goat advancement

[spiral tapir]

I mean, I was talking about the shulker boxes, but thanks for the info :p

[sand raven]

Shulker box if a perfect copy of ours, then yeah we can yeet ours and make legacy tag detector tell people to switch to vanilla

Assuming we trust vanilla not to yeet their tag like they did with music discs

[red jungle]

Caused by: java.lang.IllegalStateException: Unable to convert criterion serialization and replacement: Missing tag: 'minecraft:piglin_loved' in 'minecraft:item'
    at TRANSFORMER/[email protected]/net.neoforged.neoforge.common.data.internal.NeoForgeAdvancementProvider.lambda$replacePlayerPredicate$21(NeoForgeAdvancementProvider.java:206)

nightmare

how does this even work in vanilla

hmmm the problem is that we are trying to parse stuff in the datagen

[round perch]

The armor model/texture hooks lost a lot of context: https://github.com/neoforged/Kits/commit/c6b67e93abd131b753e1ee2707377ceebef26e40

[lucid totem]

Ok so I've figured out the strobing is related to some state that gets changed by the display window

doesn't seem to be the rest of the opengl calls in NeoForgeLoadingOverlay, I replaced it all with rendertype stuff and I still see strobing

but commenting out displayWindow.render sort of fixes it

[round perch]

Looking some more into the elytra hooks:

• The texture hook in ElytraLayer can be handled with the model parameter of the Equippable data component and a custom equipment model JSON, so I'd suggest removing it
• IItemExtension#canElytraFly() would be salvageable, we'd just need to decide whether that hook should allow bypassing the Equippable component check in LivingEntity.canGlideUsing()
• IItemExtension#elytraFlightTick() is toast unless we accept that it only gets called once every second and on a random applicable item instead of all of them

[red jungle]

keeping canEltraFly without elytraFlightTick is weird

[round perch]

Yeah

[red jungle]

dogshit but it works: https://github.com/neoforged/Kits/commit/ec50455d5aca9dd421de474e192514fa374ace93

stupid BAMBOO_CHEST_RAFT doesn't have "boat" in its name so I missed it in CapabilityHooks -_-

[round perch]

What would your suggestion be? Nuke it and let people find a way to reinstate it if they really need it?

[red jungle]

yes

[round perch]

👌

Tech, while you're looking at recipes and advancements: do you mind looking into the ShulkerBoxColoring reject? It's probably a candidate for recipe replacement as well

[red jungle]

will do

looks like a prime candidate yes 🙂

[round perch]

ItemUseAnimation is extensible now, adding back the UseAnim.CUSTOM entry with a patch is not an option anymore because the enum is part of a data component now and therefor synced

[round perch]

Hmm, ing on ChunkDataEvent.Load/Save right now and not entirely sure what to do with them. There is no point anymore where both the chunk and its serialized NBT data are accessible, for loading it would require capturing the NBT data to pass it between future stages and for saving it would require capturing the chunk in another future stage's lambda. I can see three solutions and all of them suck:

• Move the events to the place where the chunk and its SerializableChunkData are present. This prevents actually writing anything to the chunk data unless we patch an additional "custom data" compound into that record
• Move the events to the place where the SerializableChunkData is being converted from and to NBT data. This prevents accessing the chunk, so there's no way to do anything with custom data read from the chunk's NBT data and no way to get the custom data from the chunk to write it to NBT (unless you do global storage, which, uh, 🤮)
• Add additional Parse and Write sub-events that require the user to manually pass data from Parse to Load and from Save to Write, which is anywhere between really painful and actually impossible since load/save happen on the main thread while parse/write happen on the background executor

The first option is probably the least awful

Though we could of course also just tell people to use data attachments on chunks and go with the first option without the additional "custom data" tag. The benefit of the first option would be that attachments are safely accessible at both injection points since they both run on the main thread

[red jungle]

We can tell people to use attachments IMO

[round perch]

Ok, then I'm gonna finish up the docs cleanup on these events and then push option one without the custom data tag

[red jungle]

You'll notice I tend to like removing redundant hooks 😄

There's a chance we'll have to add some back

[round perch]

I do like removing them as well 😄

[red jungle]

should we override these recipes? 😄

I think so

[round perch]

Probably a good idea, yeah

[red jungle]

what about the dye + dye -> dye recipes?

I think it makes sense to also override those

[round perch]

Sounds reasonable to me

[red jungle]

OK so let's override the basic coloring recipes + the following:

        src/generated/resources/data/minecraft/recipe/cyan_dye.json
        src/generated/resources/data/minecraft/recipe/dark_prismarine.json
        src/generated/resources/data/minecraft/recipe/gray_dye.json
        src/generated/resources/data/minecraft/recipe/light_blue_dye_from_blue_white_dye.json
        src/generated/resources/data/minecraft/recipe/light_gray_dye_from_black_white_dye.json
        src/generated/resources/data/minecraft/recipe/light_gray_dye_from_gray_white_dye.json
        src/generated/resources/data/minecraft/recipe/lime_dye.json
        src/generated/resources/data/minecraft/recipe/magenta_dye_from_blue_red_pink.json
        src/generated/resources/data/minecraft/recipe/magenta_dye_from_blue_red_white_dye.json
        src/generated/resources/data/minecraft/recipe/magenta_dye_from_purple_and_pink.json
        src/generated/resources/data/minecraft/recipe/orange_dye_from_red_yellow.json
        src/generated/resources/data/minecraft/recipe/pink_dye_from_red_white_dye.json
        src/generated/resources/data/minecraft/recipe/purple_dye.json

all looks pretty reasonable to me

[round perch]

Agreed

I'll just note this in the docs of ChunkDataEvent.Save for now, but modifying data attachments in that event won't be reflected in the final serialized chunk data

[red jungle]

I removed the reject

[round perch]

👌

[red jungle]

I got to go, I'll take care of the Ingredient reject later (the single hunk that remains of it)

there's a chance that empty tags just fail to deserialize now and it's not needed anymore, we'll see

[round perch]

The empty holder set check in the constructor only handles "if right" which is the entry list, it doesn't check the tag

[red jungle]

yeah, but there's a chance that the HolderGetter rejects unknown tags

[round perch]

Ah, yeah, that sounds possible

[red jungle]

running tests needs some fixes 😄

[lucid totem]

I gave up on the overlay for now btw

[red jungle]

I can have a look soon™️ heh

but now it's time to go dancing 😄

[terse anvil]

Tech having fun instead of writing code smh

[round perch]

embeddedt, do you have an idea how to reinstate the PostChain patch?

[round perch]

You would indeed be right, it refuses to parse empty tags: [20:35:14] [Worker-Main-53/ERROR] [minecraft/SimpleJsonResourceReloadListener]: Couldn't parse data file 'minecraft:andesite' from 'minecraft:recipe/andesite.json': DataResult.Error['Missing tag: 'c:cobblestones/normal_typo' in 'minecraft:item'; Not a JSON object: "#c:cobblestones/normal_typo"': Optional[net.minecraft.world.item.crafting.ShapelessRecipe@28bb6a83]]

[red jungle]

That's great! Then we can remove the reject, and drop the barrier check in hasNoItems. I suppose we might even delete hasNoItems entirely.

[round perch]

I'm gonna leave that for later you and go play some games 😄

[red jungle]

Fair enough 😄

Enjoy

[round perch]

You too 😄

[lucid totem]

I don't fully follow how the whole postprocessing chain system works yet

(this goes for 1.21.2 and the older versions too)

[round perch]

Fair enough

[round perch]

Nevermind, I may have spoken too soon. I only tested with a non-existent tag (i.e. missing tag file) but not with an existing but empty file. The latter instead outputs this and keeps the invalid recipe: [Render thread/WARN] [minecraft/RecipeManager]: Recipe minecraft:andesite can't be placed due to empty ingredients and will be ignored

[round perch]

Yeah, I definitely spoke too soon. The recipe manager complains that the recipe is unplaceable but doesn't actually do anything to prevent using the recipe, so you get this nonsense...

[red jungle]

What is that picture supposed to say?

Empty tag ingredients should never match any stack

[round perch]

It seems to currently match anything except an empty slot 😅

[round perch]

Okay, the plot thickens: in a shaped recipe (which uses Ingredient.testOptionalIngredient() for ingredient matching) only an empty slot matches an empty tag file, but in a shapeless recipe (which uses StackedItemContents#canCraft() for ingredient matching) anything except an empty slot matches an empty tag file

[round perch]

Nametag render event reinstated: https://github.com/neoforged/Kits/commit/56d2444742b52ba35629df1cddaa2f27dfe811d2. The event names are kinda meh, so if anyone has a better idea, feel free to change it

[drifting ice]

Sorry for late primer, it should be ready sometime within the next 24 hours, assuming I don't get stuck on something again

[red jungle]

Pfff they did it again

[red jungle]

reproduced yeah

wtf

[timber jolt]

didn't we have a similar issue before?

[red jungle]

it's mojang's fault most likely

[round perch]

I wonder whether there is a Mojira ticket for this since it should be reproducable in a completely vanilla game with just a datapack

[red jungle]

they keep getting it wrong

[timber jolt]

thinking about https://github.com/neoforged/NeoForge/issues/841

[red jungle]

yes that was one of the mistakes in the 20.2 custom ingredients implementation

which we did not do again

[red jungle]

no idea, let's see

https://bugs.mojang.com/browse/MC-277027

[dull heron]

They should really just let ingredients be, lol

I wonder what motivated this new refactor anyway

[round perch]

Does the related issue for shaped recipes accepting exclusively empty have a ticket?

[round perch]

Likely the ability to send tag ingredients as tag keys instead of resolved lists over the network

[tacit arrow]

That would be nice

[lucid totem]

as usual the Mojira mods don't actually read and just close the issue if it doesn't follow an exact template

sigh

[toxic river]

no... just edit the bug report... it's automatically reopened

[round perch]

The point is that the required information is present, it's just not done exactly the way the template specifies

[toxic river]

1. (Explain what needs to be done for the issue to happen)
2.
3.

Observed Results:
(Briefly describe what happens)

Expected Results:
(Briefly describe what should happen)```

[lucid totem]

Templates are mostly for regular users, the average developer will provide enough info even without a template

[toxic river]

Normally, in a bug report, there's always the datapack to reproduce the bug...

and a video/screenshot

[red jungle]

Bruh dumbass mod

[kind fractal]

attached a datapack and a correction to the report

[red jungle]

Thank you

[drifting ice]

Okay done. Mainly boat restructuring and levels being replaced by server levels

[red jungle]

wdym?

why would an empty slot match an empty tag file -_-

[round perch]

Because it does, ask Mojang how that makes sense

[red jungle]

...

I can't reproduce it (with shaped recipes)

and fwiw the following test is false:

(the ingredient is an empty tag)

this is false too

[round perch]

Hmm, I can't reproduce it anymore either. No clue how I managed to make it happen

[timber jolt]

we do love some heisenbugs

[ancient frost]

try

• first session in a brand new world
• first session in a brand new world after /reloading
• existing world
• existing world after /reloading

[fathom compass]

what would happen if we changed recipes a bit more

[heavy raven]

lol

oh today is tuesday 😮

[cosmic acorn]

depending on the changes, some people may be very happy, or very annoyed

[clear urchin]

well, as long as it is not world gen and tree gen

[cosmic acorn]

;P

[lunar dagger]

now i'm tempted to change the role icon for Problem Causers to an explosion

[heavy raven]

proceeds to overhaul recipes

[red jungle]

oh no

[short iris]

[Reference to](#mojira message) #mojira [➤ ](#mojira message)This ticket has just been resolved as Fixed!

technici4n referenced · Oct 1, 2024, 9:41 AM

Patbox

[MC-275706] Vanilla clients can't display items with components in recipe book anymore

In 1.21.1 and before, ingredients of recipes were synced as an list of Item Stacks (with all components / nbt data) which was changed in 24w33a to only sync item types. While this functionality was never used by vanilla server, custom servers (and plugins/mods) used it to show custom ingredients (with all their components) in recipe book's ghost slots/grouped recipe preview. As ghost slot displaying (but not how it's displayed) and actual slot matching logic is running server side, there were no notable side-effects of components being present on the client (aside of visuals that is). With this functionality no longer being present, recipe book loses it's usability on servers that add a lot of custom content with custom recipes.

Status

Resolved as Fixed <t:1727775678:R>

Fix Version

Future Update

Assignee

[Mojang] Bartosz Bok

Votes

22

Comments

3

Created

<t:1724143326:R>

Quoted

[red jungle]

I bet this is related 😄

let's see if we can get rid of this

[spiral tapir]

Inb4 ingredient predicates

[red jungle]

imagine

[red jungle]

I think I found the explanation for the flashing screen

the problem happens when rendering the mojang logo

earlydisplay does glBindTexture(GL_TEXTURE_2D, textureId); and glBindTexture(GL_TEXTURE_2D, 0);

however, blaze3d isn't aware of this, so a call to RenderSystem.bindTexture(...) might not update the texture

you know why it flashes?

because every client tick (not frame) the active sprites are uploaded

which causes a different texture to be bound, and therefore forces a rebind

so every client tick you get a single red frame, then it's back to black

fixed!

    /**
     * TODO: does it make sense to keep this?
     * ItemStack sensitive version of {@link Item#getEnchantmentValue()}.
     *
     * @param stack The ItemStack
     * @return the enchantment value
     */
    default int getEnchantmentValue(ItemStack stack) {
        return 0;
    }

can probably be removed

in the spirit of the other stuff that got data component-ed

[round perch]

I wanted to ask @dull heron about that since he's mister enchantment.
If you remove it, don't forget to delete the EnchantmentHelper reject.

[dull heron]

probably fine to remove that yeah

[round perch]

👌

Yeet

[ancient frost]

brewing recipes???

[sand raven]

TNT CRAFTING

[random tangle]

Those still aren't data-driven?

[ancient frost]

haha no

[random tangle]

wild

[ancient frost]

and every time someone tries to add datapack brewing to fnorge, it turns out to be a big can of worms, and they decide that the worms are better off being inside the can and put the lid back on

[random tangle]

In what sense?
Making them data-driven is inherently complicated, or just patching it into Minecraft instead of Mojang doing it is complicated?

[ancient frost]

the fundamental issue here is that brewing has to work on neoforge-vanilla and vanilla-neoforge connections

[random tangle]

ahh

[vivid ore]

Plus the issue that the way brewing recipes work in vanilla is... already its own can of worms before you even start attaching datapack stuff to it

[dull heron]

the way brewing recipes work in vanilla is actually remarkably friendly to moving it to a data system

the main issue is just keeping vanilla compat

Placebo retains the ability to add additional brewing mixes via datapack, but does not make any attempt to interact with the existing ones

[vivid ore]

well... sorta. Neo's brewing recipe API makes it not terrible

But without that there's a lot of fun hardcoding going on

[ancient frost]

well, we also hardcode all the recipes when we use the brewing api

[vivid ore]

Yes but you don't severely limit what the input item can be at least

[dull heron]

Vanilla doesn't strictly limit it either, it just retains a List<Ingredient> of valid containers

[tidal siren]

That but instead of the two TNTs, its MOJ and ANG

[bronze crescent]

done

[ripe drum]

that's perfection

if someone doesn't snipe me ill add it in the morning

[vivid ore]

I love it

[random tangle]

tempted to throw this in a server resource pack and see how long it takes people to notice

[bronze crescent]

You'd need more than just the texture, the TNT model shares all 4 sides

But I did this in Blockbench in like 10 minutes, so.. simple enough

[timber jolt]

Fairly simple change tho

Just gotta change the texture reference for two sides

[tidal siren]

rotate it 90 degrees and read it as ANG MOJ

[timber jolt]

time for custom renderer

[spiral tapir]

Late snapshot today

[wise mica]

i wonder if we get the new features

they were pretty complete at live it would make sense to add them now behind an experiment

[bronze crescent]

#blameBoq or something

[wise mica]

boq pushed a recipe change and now the game won’t start 😔

https://tenor.com/view/ren-and-stimpy-button-red-button-gif-11093706934151371252

[red jungle]

just disable recipes for the snapshot, what could go wrong

[round perch]

[spiral tapir]

Wouldn't be the worst thing. Though, might as well change the title screen to just say "Mine" at that point

[clear urchin]

WAIT FUCK

I DID NOT EVEN REALIZE

I LOST THE BET

WEEEEEE

[inland vault]

Minecraft Snapshot 24w40a

Hello!In this week's snapshot we are adding a new experiment which will allow you to experience the Pale Garden, a new eerie biome filled with Pale Oak Trees and Hanging Moss. Beware of its sole inhabitant, the Creaking, and don't blink!Happy Exploring!Exp...

Oct 2, 2024, 2:54 PM

[haughty raven]

<@&1067092163520909374> ^

[spiral tapir]

There it is

[lunar dagger]

IT BEGINS

[prisma pawn]

[grand crypt]

woooo

[prisma pawn]

it begins

[inland vault]

New version detected: 24w40a.

New version detected: 24w40a.

[grand crypt]

yeah we now @inland vault

[prisma pawn]

/rotate??

[lunar dagger]

WHAT

[inland vault]

New version detected: 24w40a.

[lunar dagger]

we can rotate entities via command now 🎉

which means we can rotate players

(without teleportation tricks)

[drifting ice]

Couldn't you do that already?

[lunar dagger]

iirc, no...?

[drifting ice]

I thought that was possible through the data entity set command

[lunar dagger]

at least, without constantly teleporting a player to a player an entity

players are the one exception to the data command

you can read, but can't modify

[prisma pawn]

unfortunately

[drifting ice]

ah got it

[wise mica]

big feature snapshot this week

not a lot of tech changes

😔

[drifting ice]

I prefer that

It reduces primer time from like 10 hours to 2

[tacit arrow]

Is it really different than teleporting with relative xyz and then different rotation? Pretty sure it has a sync variant for packets that only really does rotation

[haughty raven]

https://github.com/neoforged/Snowman/commit/7ebe2a8929d4f12098dc7c0ea873d3f40ac32df7
for those with snowman perms

[lunar dagger]

you're telling me it finished already!? impossible /j

[dapper pendant]

Well not that they stated 🙃

Or did you look at the diff?

Bevause they could've rewritten an entire system

Oh boy!

[wise mica]

i can’t see the diff unfortunately

[lunar dagger]

quite a chonk

[bronze crescent]

oh lord

[prisma pawn]

such a shame I can't /ride leash knots anymore

[bronze crescent]

Lots more realms stuff..

[dapper pendant]

Remember that boq said something about recipe changes

And there do seem to be a fair number of those

[drifting ice]

Recipes have resource keys now

[clear urchin]

so where is the blog post

[drifting ice]

So they're probably in the same realm as loot tables

[bronze crescent]

RecipeBookCategories is gone o.o

[grand crypt]

this sounds interesting.

[lunar dagger]

hopefully that's somewhat generalized

[wise mica]

Oh please tell me they turned recipes into a proper registry that would be great

[grand crypt]

added trail particle with the following options:
color - the color of the trail
target - the position of the target for particle to reach
this gonna be good for mapmakers

[dapper pendant]

Apparently the new tag is what controls the pumpkin overlay now

[grand crypt]

hopefully

[bronze crescent]

o.o

[spiral tapir]

oh

[prisma pawn]

[bronze crescent]

Direction now has getPositive and getNegative methods

[prisma pawn]

nice

what does "slot display" mean in the context of a recipe though

[cosmic acorn]

huh, I did not expect them to do this as an experiment

[bronze crescent]

WardenEmissiveLayer is now LivingEntityEmissiveLayer

Mob mods rejoice on that one

[grand crypt]

I love generalization

[drifting ice]

Well, rewriting the recipe book extension is going to be annoying

They're back to using static final fields again

[bronze crescent]

oh thank god they finally split up renderItemDecorations

It now has renderItemBar, renderItemCount, and renderItemCooldown methods

Kinda wish those were static and took in the pose, but progress

[haughty raven]

why must they be private tho, neo for sure gotta at those public

[bronze crescent]

Because they use state

pose and such

[dapper pendant]

Or, well, Direction.Axis

[spiral tapir]

oh, so are recipes a reloadable registry now?

[dapper pendant]

And it has getDirections now too

[digital needle]

Is it known if this is data driven in datapack or just plain code?

[dapper pendant]

No, they're a fake registry, like functions

[spiral tapir]

[ancient frost]

I assume it's just done in animatetick like fire

[tropic anchor]

18 compile errors for neoform

[wise mica]

they probably made it data driven

fingers crossed

they don’t hardcode much these days since they want fully data driven blocks

[red jungle]

    public SlotDisplay display() {
        return (SlotDisplay)this.values.unwrap().map(SlotDisplay.TagSlotDisplay::new, $$0 -> {
            return new SlotDisplay.Composite($$0.stream().map(SlotDisplay.ItemSlotDisplay::new).collect(Collectors.toUnmodifiableList()));
        });
    }

Only change to ingredient

Pretty cool though

That's exactly what we need for AE2 etc

Actually it's annoying that we need to manipulate both the slot displays and the items in compound ingredients

[round perch]

In case you want to update Kits to 40a tonight, I won't be able to help you, I'm out with friends

[drifting ice]

Recipe proprety sets are just...why right now

They need a transformer or something because it's just hardcoded nonsense

[dapper pendant]

Well, moved

And reworked

[split wharf]

according to patbox, recipes are no longer synced on the network

I think it's to the benefit of a lot of people if neoforge reimplements that and enforces the contract of network serialization on recipes

[dapper pendant]

I can confirm that recipes are no longer synced in full, but some data about them still is. E.g. ingredients, size (for shaped recipes), result, etc.

But for recipe viewers that's definitely not enough info

[split wharf]

in particular, mods like JEI and EMI need this data for plugin developers to have a reasonable time developing their plugins

[dapper pendant]

Well it is for shapeless, but not shaped

[split wharf]

it would really negatively impact the workflow if modders themselves needed to manage network syncing for obvious reasons and would also be bad if recipe viewers had to make their own implementations, the latter particularly for duplication issues and requiring the mod on the server (instead of just neo)

[ancient frost]

jei can just sync the full recipes themselves I guess

a lot of jei isn't even recipe recipes

JeiRecipeCodec

though it would have to run serversided

I didn't read the post above mine and they said everything I did first

[dapper pendant]

I agree with Emi that it would be good for NeoForge to implement this itself

[split wharf]

recipes don't have a serialization guarentee, or at least won't soon

I believe they still have a deprecated one

for the current snapshot an implementation of JEI or EMI can sync, but I have very little faith it will remain, and if both are loaded get ready for 2 massive wasteful packets

[dapper pendant]

I mean, a Codec is enforced, because that's how they're parsed

Just the StreamCodec may go away soon

[split wharf]

the maintenance burden is pretty small, hopefully, neo implements the vanilla recipes based on current vanilla sync, an abstract method is added and modders only need to migrate to the new serialization before they notice it's gone

[terse anvil]

Random restriction is random

[split wharf]

the codec is deprecated according to patbox

[dapper pendant]

The StreamCodec is deprecated

The Codec is not

The Codec is how recipes are loaded from json files

[split wharf]

I'm gonna be real I don't touch codecs do they also handle packet serialization

[terse anvil]

No

StreamCodecs do

[split wharf]

aha, then yes that

[terse anvil]

But you can use a Codec to serialize

[dapper pendant]

You can serialize an object to a packet using a Codec

It's just less space efficient

[terse anvil]

You just have to convert to NBT and then send the full NBT structure

[spiral tapir]

they shouldn't, at least, you can send it as json/nbt through the friendly byte buf

[terse anvil]

But bandwidth goes woooo

[split wharf]

that packet already explodes on modest instances

[terse anvil]

Then again, I don't think Neo should sync recipes to the client just because

[split wharf]

at worst just add a call if a mod wants recipes synced, I guess

[terse anvil]

Why not just have *EI installed on both sides?

And have them sync it themselves?

[split wharf]

both JEI and EMI are installed in most modpacks, and both would likely sync these themselves, the average case network cost is higher

[terse anvil]

Isn't EMI installed on top of JEI?

[split wharf]

no? it's a standalone mod

[terse anvil]

Why would you install both then?

[timber jolt]

codecs are also just slower

[split wharf]

JEI hands over plugin registered recipe data

[dapper pendant]

The recipe is still sent it seems actually, just in a completely different way

[terse anvil]

So EMI doesn't have to do anything

JEI hands over data 😛

[split wharf]

EMI functions fine without JEI

it in no way depends on it, and I can't expose JEI's api through my own

[terse anvil]

"Is Jei installed? Don't sync"

Problem solved

[split wharf]

this does not solve the problem of stream codecs going away

[terse anvil]

Codecs exist

It'll just take ages to send stuff

But it can be done

[split wharf]

that's bad

[terse anvil]

Sure

[split wharf]

how many neoforge instances run without JEI or EMI

[terse anvil]

But I don't think that enforcing a random restriction is good either

Your APIs could very well require registration of a StreamCodec

[split wharf]

at worst you can allow mods to opt in to serialization and then if they can't access their recipes on the client it's their fault

[dapper pendant]

The recipe is still sent

[terse anvil]

Oh well, then no problem at all

[dapper pendant]

It's just sent differently

Instead of the recipe being sent directly, RecipeDisplay objects are sent instead

It's basically a stripped down version of a recipe that eliminates some server-specific info, such as advancement data

[spiral tapir]

yeah, I don't know exactly what recipe viewers need from the recipes, but there's a lot of display info still sent to clients

[dapper pendant]

The notable difference is that SlotDisplay objects are sent instead of Ingredient objects

You can think of them like client-sided ingredients

[spiral tapir]

arguably nicer

[timber jolt]

which morning did you mean

[dapper pendant]

Yeah they're pretty nice

[split wharf]

plenty of recipes lack these, though

in vanilla, campfire cooking at least

and stuff like smithing is just lacking any of the input information

it also looks like recipe displays don't have ids...

[viscid gyro]

stream codecs are going away ?

[timber jolt]

aiui, no, just recipe sync changed

[red jungle]

We need to finish the 39a rejects first, and make sure the automated tests pass at least

[drifting ice]

And we're up: https://github.com/ChampionAsh5357/neoforged-github/commit/479c0cbac1e0de1d265ee26c7ef2c1bde28869c0

Basically tons of recipes changes

And teleport method renames

[bronze crescent]

did you forget

[dapper pendant]

Oh yeah, not great

It would be nice if NeoForge synced this info

[ripe drum]

yes

[lucid totem]

I don't see anything wrong with requiring recipes to be serializable via codec/streamcodec, but I don't know if Neo force-syncing recipes unconditionally is a good idea. There are certainly people that choose to play without a recipe viewer; there's no point forcing them to incur the overhead of deserializing recipes

[split wharf]

it is fine if there's an api call to trigger the sync

[lucid totem]

But even that has implications, e.g. it allows hacked clients to bypass limited crafting or whatever the gamerule is called

[split wharf]

uh, I don't think so? crafting is still handled by the server

[lucid totem]

I don't know if anyone in modded actually relies on hiding recipes for fairness

[lucid totem]

Yes but in the current snapshot the client doesn't know the full set of recipes if they're locked, correct?

The hacked client would have to try crafting literally every possible combination by force, and could only do so with stacks the player has

[split wharf]

not sure but that's even more reason to sync recipes

[fathom compass]

uh, no, if limited crafting is on, server will not recognize a recipe that player does not have in recipe book

[stiff canyon]

Yeah, the RecipeManager IIRC specifically checks if the player has access to the recipe before returning it

[fathom compass]

what changed in snapshot is that the client will only receive the recipes in recipe book, limited crafting or not

So only way in vanilla to get all recipes is /recipe give @s *

[red jungle]

Oh so we need to sync machine recipes that are not in the recipe book now

[stiff canyon]

Or add them to the recipe book

[ancient frost]

IIRC the main reason mods don't use the recipe book is we don't have patches to make displaying 400 different types of recipes nicely like we do the creative tabs

[fathom compass]

You could add a packet for a client to ask for all recipes displays

[ancient frost]

and nobody ever implemented that because jei/emi/fmi exists

[red jungle]

For sure yes

[stiff canyon]

This is IMO a good thing long term - clients shouldn't need to have access to literally everything under the sun. Recipe viewers can either include an optional server component if they really want to "sync recipes like it used to be", or just dynamically build their "recipe tree" based on what recipes the client has unlocked, listening for recipe unlocks from the server

[red jungle]

That might be somewhat of a lazy solution though 😄

[stiff canyon]

Also not compatible with vanilla - I suspect if something like JEI goes that route, players will be opening bug reports asking "why can't I look up the recipe for an iron chestplate?" when they don't even have iron ore

[timber jolt]

yeah, that's gonna be annoying

[sand raven]

The recipe viewer showing only unlocked recipes sounds interesting tbh

[timber jolt]

sometimes you have an end goal, but no clue how to get there

[sand raven]

And would stop issues where servers try to hide recipes as a mechanic where recipe viewers now cheat through

[red jungle]

It sucks for heavy modded

[stiff canyon]

It's been a thing in a bunch of mods for a while; Thaumcraft did it a lot for example

[red jungle]

Depends how it's set up with limited crafting

[timber jolt]

I mean, if it's gonna be part of the optional server components for those mods (perhaps with a config option), I think it's fine, it just shouldn't be removed outright

[sand raven]

How are recipes discoverable right now

Advancements?

[stiff canyon]

Yeah, Advancements unlock recipes

In any case, I suspect the only real solution is to build one recipe graph for the client recipes, and then build the graph of server recipes based on a view of those client recipes, with the appropriate overrides from the server, and somehow indicate to the player whether a recipe comes from the server or the client

(at least for recipe viewers that want to show "all recipes", even locked ones)

[sand raven]

The advancements can lead to lots of spam by lots of mods. Could neo do a more efficient system where you can mark a recipe as always discovered on client

[ripe drum]

[timber jolt]

very high res, yes yes

[ancient frost]

are recipe advancements mandatory now

we need less of those

[lucid totem]

Every recipe has always generated an associated advancement iirc

[ancient frost]

no?

[lucid totem]

I thought someone told me this the other day

[timber jolt]

you don't have to, but you get warns iirc

[sand raven]

Only in datagen

[lucid totem]

Ah

[ancient frost]

oh, the vanilla recipe provider might require them, I don't use that though

because it requires advancements

recipe advancements are just extra cruft sent to the client in modded, nobody's ever asked me to add them for my mods

[sand raven]

I had people ask me to fix my mod’s recipes in recipe book

So yes people look at book in modded

But the question is, how much tradeoff are we doing if we make all mods required to have advancements for their recipes

We just traded recipe packet spam for advancement packet spam plus inventory scanning for the advancement

[fathom compass]

It's like 5 minute job to make some recipes always available
Just make it opt-in
Vanilla just never had a use case for it

Well, maybe except for that magic crafting table recipe

[stiff canyon]

magic crafting table recipe? O.o

[sand raven]

Crafter secret inside name at Mojang

[fathom compass]

yeah, it's a magic coffee table at out office where we do mines and crafts

[weak falcon]

arts and crafts

[red jungle]

I don't like the idea of going back to the old behavior when you clearly wanted to change how things work

Maybe we'll do it, but it makes sense to explore the solution space a bit first 😄

I need to check how the new system handles syncing non-item data (e.g. fluid inputs/outputs for a recipe, or energy usage, etc)

[dapper pendant]

SlotDisplay is the client equivalent of Ingredient

[fathom compass]

The goal was to not send all the recipes all the time
Opt-in is fine. Even all recipes is fine, though you have probably hit packet size limit multiple times

[dapper pendant]

NeoForge has support for spreading out packets over multiple packets. Or at least it was planned

[red jungle]

We have automated packet splitting for that

[ancient frost]

I think looking into patching better mod support to the recipe book is a good first step

it's been something people have been poking about for years now, it's just... nobody's needed to really do it yet

[fathom compass]

Yeah, split packets were always there
"mojang: 4 MB compressed should be enough"
"modders: I see that as a challenge"

[ancient frost]

and then like, orthagonal to that, people have also been poking about making some sort of unified api for recipe viewers

[dapper pendant]

Yeah resourcepack sending will be fun for one of my mods, heh (though I'll probably do some kind of proxied web server instead, as that works with Vanilla clients)

[ancient frost]

and the reason we don't have that is it turns out there's always less common ground between recipe viewers than everyone assumes at first

but now maybe the recipe book can be the common ground?

[split wharf]

lacks way too much information to be a common ground

[fathom compass]

eh, not that much has changed, instead of recipe server sends a display, which is just bit of hint for player about recipe without constraints of being actually craftable.
So instead of sending ultrapowertechmagicmod:unltimate16x16recipe type, just send "yeah, it's a shaped 16x16 recipe, those are inputs, those are outputs", anyone wants to display that?

[red jungle]

how do I send the power consumption etc

[split wharf]

just serialize it

[sand raven]

power is the number of slots

[dapper pendant]

Well crafting_shaped is a type of RecipeDisplay, so you'd create your own RecipeDisplay that also sends that information

[ancient frost]

recipedisplays sound typedispatched?

[dapper pendant]

minecraft:recipe_display registry

[fathom compass]

make a recipe display type, register, make a recipe that returns it, fire it over network and hope something on client side is willing to render it

don't have to limit stuf to slots, it's upcasts all the way

[ancient frost]

hey, mojang made a common recipe viewer API before we did

[fathom compass]

ooop, sorry, "patterned switch"

[red jungle]

ohhh I see

[dapper pendant]

I mean, it kind of is, it's just missing the ID

[weak falcon]

minus the viewing part, apparently?

[ancient frost]

it's an api, not an impl

[red jungle]

that's quite cool actually

[ancient frost]

well there's also an impl I guess

[red jungle]

recipe viewers can probably switch to using the recipe displays 🤨

[split wharf]

no, they lack information

I will continue saying this

[weak falcon]

what information are they lacking?

[dapper pendant]

What information do they lack other than a persistent ID? Which they do definitely lack

[split wharf]

id, remainders, and implementation specific data like cook time, ingredients for smithing recipes, every campfire recipe

[ancient frost]

remainders are part of Items, not Ingredients

[split wharf]

not in practice

[sand raven]

If a mod has a recipe that destroys the item rather than returning the remainder, sounds like a bug

[split wharf]

no? what if you want water bucket + spider eye -> bucket of wet eye

[ancient frost]

yeah I just updated jumbo furnace a couple months ago to respect the remainders... in items

[split wharf]

people make recipe specific remainders all the time

[ancient frost]

"recipe specific remainders" are just "a second output"

[weak falcon]

wouldn't those things be part of the recipe display registration?

[split wharf]

not for reasonable display metric

[red jungle]

implementation specific details would certainly be in the display

although... FurnaceRecipeDisplay doesn't seem to send the burn time

[ancient frost]

if we need implementation specific details that aren't easily retrievable by the subclass then we can just do something like IRecipeDisplayRenderer<T> like menuscreens

[fathom compass]

yeah, and the way to get remainder info is to let the recipe tell via display you and not dig into every actual recipe implementation
since the only interface right now is "getReminderAfterCrafting"

[weak falcon]

the burn time is constant though, isnt it?

[ancient frost]

no

[split wharf]

anvil smithing recipes only send the output

[ancient frost]

furnace recipes can specify how long it takes to cook the furnace recipe

[weak falcon]

how have I missed this...

[dapper pendant]

That is data that is not sent that would be useful to receive

[red jungle]

anvils use recipes now??? 🤨

[fathom compass]

smithing tables

trims and netherite

[ancient frost]

if vanilla doesn't need it we could patch it in

actually no

that would nuke vanilla connections, probably

[fathom compass]

Current contents of displays are based on vanilla needs, doesn't mean they can't change
I'd like to eventually display cooking time in vanilla, but it turns out tooltips are hard

[red jungle]

makes sense

in principle nothing stops mods from including in the RecipeDisplay whatever info they want to use on the client-side JEI/etc integration

[split wharf]

you can't really add stuff to vanilla displays without causing compat headaches, but yes there is nothing stoping mods from including their recipe id and all required info in their displays

[red jungle]

vanilla displays might need special tricks, or a side-channel

[ancient frost]

and then you just give jei their IRecipeDisplayHandler<T> or whatever

[split wharf]

devs will still need to handle the hookup between whatever recipe representation and recipe viewer data though

[red jungle]

yes

[split wharf]

re technici4n

[red jungle]

@round perch do you know why this even exists?

like... what's the point?

[split wharf]

I guess re "this is a recipe viewer agnostic api"

[red jungle]

looks like registry replacement leftovers to me

[red jungle]

yeah you still need the RecipeDisplay -> EmiRecipe or whatever mapping to be provided by the mod

[red jungle]

hmmm sadly this is still needed for registry sync stuff

we still have these rejects

many of them are related to removeEffectsCuredBy

maybe we keep these for 23w40a; I'll update Kits tomorrow

[round perch]

The remove method is deprecated for removal, it was indeed related to registry replacement. The add method is there to fill the block-to-item map in our registry callbacks. We might be able to fix this in a similar way to other "registry submaps" where we replace the vanilla map with ours instead of using a separate map, though I'd have to look into the specifics tomorrow.
Edit: just saw your fix, that looks fine as well

[round perch]

I'm honestly considering ripping the cure system out. I don't see any way of implementing that without breaking vanilla network compat

[ripe drum]

#566276937035546624 message hmmmm

[wise mica]

https://tenor.com/view/typing-jim-carrey-jim-carrey-type-gif-4680550

boq going to completely break recipes again because we're complaining

next week they'll be a whole new type of registry and JEI wont be conceivable

[heavy raven]

JER

Just Enough Recipes 😄

[red jungle]

I will add this to the build.gradle:

tasks.register("genPatches") {
    dependsOn tasks.unpackSourcePatches
}

25 new reject files for 24w40a

finally they got rid of hasCraftingRemainingItem

now they just check if getCraftingRemainder is empty or not

[fathom compass]

TODO comment above that method was only 8 years old, give us a break

[red jungle]

now time to get rid of separate matches and assemble? 😄

[manic crescent]

I'd almost be tempted to say we should add it back if they do

like, match gets called a lot, and assemble might be expensive for custom recipes

[fathom compass]

I'll make a TODO

[ripe breach]

isn’t there already one?

[red jungle]

see you in 8 years

[grand crypt]

why are y'all speaking like this?

[tidal siren]

why not™️

[grand crypt]

good point

[scarlet pawn]

Because emphasis is nice

[grand crypt]

isn't this emphasis

[ripe breach]

this is how you know the conversation is very serious

[grand crypt]

I see

[cosmic acorn]

in old html <em> did italics, and <strong> did bold

to me a full line in italics is an aside

while a single word of phrase is emphasis

I went to that place

[scarlet pawn]

That's also my take

[fallow nest]

Define “old”? It’s still that way isn’t it?

[heavy raven]

Holder.is(Holder) is deprecated

[lunar dagger]

Holder.equals perhaps?

[blazing steppe]

I don't think holder has an equals implementation

[red jungle]

we patch it in

we need the real holder and the DH holder to be equal

[blazing steppe]

Yea but the deprecation is vanilla

[cosmic acorn]

it still works, yes, but I don't think it's recommended to use those tags since they are not well-defined in terms of what the tag should look like

[fallow nest]

I think that’s the point isn’t it? If you want to emphasize something put it in <em>

And then the web designers decide how emphasized stuff looks

[cosmic acorn]

looking at the docs, looks like it is useful, for screen readers

so the voice speaks with emphasis too

[fallow nest]

Yup

There’s also <i>

[cosmic acorn]

I guess <em> is still useful

if you want actual emphasis and not just visual italics

[fallow nest]

Correct

[fallow nest]

<i> is italics without semantics