yeah
#1.21.2 Snapshots
cosmic acorn
I guess the short of it is: if you want italics use <i> or css, if you want emphasis use <em>
and similarly, if you want bold use <b> or css, but if you want it to be spoken more strongly, use <strong>
lunar dagger
i originally thought <i> was fully deprecated and not for use anymore
then I learned that it's used for italics where the semantics of emphasis are inappropriate
like italicizing scientific names, or foreign words
(although for that last one, something something lang attribute)
fallow nest
Yes and then thereβs also <abbr>
With an optional attribute title
Thatβll make screen readers pronounce it differently too etc
lunar dagger
I use <abbr> in my student email signature 
fallow nest
Since accessibility is a legal responsibility for German websites I know all about that 
lunar dagger
inaccessible websites will be made inaccessible in Germany /j
fallow nest
By fining the responsible people to death yeah
ancient frost
if jei and emi aren't viable anymore someone will just make a new recipe viewer api
all of this has happened before, all of this will happen again
Finally Enough Items
heavy raven
JEI will still exist pretty sure 
terse anvil
Bunch of Recipe Information
sand raven
Finally Enough Items <:thinkies:932605814995292192>
Sectioned Held Items Together
grand crypt
Definitely A Lot Of Items - DALOI
red jungle
hmmm the Ingredient StreamCodec hasn't been deleted yet 
however, it doesn't seem used in practice anymore
our ingredient patches will probably have to wait for the next snapshot
manic crescent
that one april fools update that removed items should have been called NMI - no more items
velvet spindle
on the topic of crafting and recipe books. what does vanilla do if limited crafting means the client doesn't have the recipe, but the user configures an auto-crafter to make the item?
ancient frost
I always assumed autocrafters just ignored recipe locks
and that packs/maps that lock recipes would just lock the recipe for autocrafters
velvet spindle
π€ that seems like an oversight.
like, couldn't the crafter just store the userID that placed it (or keep tabs of who accessed it) and then only enable recipes on the server that match their unlocks?
this way unless you come across a crafter placed by someone with more unlocks, you can't craft more than your recipebook would have let you anyways. and that would open up some cool map features where fakeplayers could be used as recipe containers on maps that make in-world auto-crafters into special crafting stations.
neat galleon
Then you'd end up with the reverse issue (if you have a recipe your friend doesn't have, if they placed the Crafter, you can't use the recipe)
velvet spindle
(or keep tabs of who accessed it)
ideally anyone who opens the crafter would have their unlocks included
neat galleon
My bad, missed that! That said, now you still have someone crafting a recipe they don't have, as long as someone else has it. Or also issues where the person with more recipes can't craft a recipe if they decide to load the Crafter via automation (thus they aren't interacting with it)
I think the current situation is less of an oversight and moreso they had to compromise somewhere, and they chose to let people auto-craft recipes they don't have, then have situations where you couldn't auto-craft a recipe you did have
velvet spindle
I think if you can't open the auto-crafter gui, then that's an intentional map design. otherwise, open the gui to add your recipes to the crafter.
The only thing i thought of with that design that might give pause is, you either have the crafter store the distinct recipes of everyone who opens it, which seems excessive. or you store the player IDs and ask the server what they have unlocked, which means the crafter can gain new recipes if the user learns something after they've opened it. I don't think either is bad since they both have advantages and drawbacks, but complicates things nonetheless
manic crescent
do furnaces do recipe locks?
like it's not a fundamentally different question
[pretty sure they don't]
like, they have a pretty elaborate system for granting recipes to players [which really overcomplicates how xp is managed, since they're tracked together], but they don't iirc have anything to manage "what is this furnace allowed to smelt"
red jungle
isEnderMask needs to be tweaked to also work with Creakings
looks like it works in the opposite way 
red jungle
maybe red just needs to be replaced by SearchRecipeBookCategory, but I have no clue what the code is doing
round perch
Yeah, that's going to be interesting
red jungle
hardcoded enums as usual!
round perch
At least we can make the enums extensible fairly easily, so it's not as bad as the "recipe property sets" loading in RecipeManager
dull heron
real fake resource keys
ancient frost
ah, like Levels!
drifting ice
Oh, the recipe book stuff changed massively
tropic anchor
Seems like only BasicRecipeBookCategory and SearchRecipeBookCategory need to be made extensible and RecipeBookManager can be removed since SearchRecipeBookCategory replaces AGGREGATE_CATEGORIES, Recipe has the method recipeBookCategory which returns a BasicRecipeBookCategory for RECIPE_CATEGORY_LOOKUPS, and RecipeBookComponent accepts a list of RecipeBookComponent.TabInfo as part of its constructor which includes icons and a RecipeBookCategory for TYPE_CATEGORIES
drifting ice
Personally, I would rather not touch the enums at all and build a separate system around a custom RecipeBookCategory so that people can make more extensible systems
Only would allow modifications of the existing aggregate categories
red jungle
It's easier to make a few enums extensible than to implement a separate system
drifting ice
I mean, yes, but that doesnβt mean I agree it should be done that way.
Well, itβll be done in whatever way you and porters think it should be
red jungle
is this expected in this snapshot?
drifting ice
No, they should exist?
haughty raven
yeah, just booted up vanilla and they do exist
https://github.com/misode/mcmeta/blob/8109fc30a808c4fc5df4b04bd8ba79b95526b1b6/assets/minecraft/lang/en_us.json#L1884-L1901
so seems like something we did broke them?
lunar dagger
Bootstrap has code to detect if there are untranslated strings and error out in dev
haughty raven
thats whats logging it, Bootstrap#validate iirc
lunar dagger
presumably, so Minecraft developers can't accidentally ship features while forgetting to add the proper English base translations
red jungle
I suppose the DFU warnings are also not expected?
probably just forgot to sync intellij then, let's see
yeah that was it; thanks for checking!
one day I'll fix
[17:34:21] [resourceLoad/WARN] [minecraft/ModelManager]: Missing textures in model neoforge:bucket_milk#inventory:
minecraft:textures/atlas/blocks.png:neoforge:items/bucket_base
minecraft:textures/atlas/blocks.png:neoforge:items/bucket_cover
minecraft:textures/atlas/blocks.png:neoforge:items/bucket_fluid
moving on... looks like BlockEntityType.Builder is gone
haughty raven
yeah was dropped in a previous snapshot iirc
just invoke block entity type ctor now with the factory, valid blocks and dfu type
rather than going through the old builder
red jungle
DFU type is gone and the ctor takes a set
I suggest we add BlockEntityType.of(factory, Block...)
haughty raven
why not another ctor which just this()s to the vanilla passing the vararg as a set?
red jungle
yeah that works
haughty raven
that way method references for both would be BlockEntityType::new
which i would prefer personally
red jungle
should I throw if someone passes an empty set?
vanilla has a warning in register at least
red jungle
it's super easy to forget in the vararg at least
vivid ore
So presumably someone might set up nothing to start and add with the event
...On the flip side, a warning isn't a bad idea since it's an easy footgun with the vararg. What's that vanilla warning look like?
red jungle
I think I'll do this
vivid ore
Ah, throw for the varargs but not the set; that makes sense
Should add some comment to that extra ctor that it's neo-added
red jungle
true
red jungle
InteractionResult.sidedSuccess is replaced by InteractionResult.SUCCESS apparently?
ancient frost
I was using SUCCESS already so I don't have to refactor
cosmic acorn
I never used sidedSuccess
I used success/consume wherever it felt like vanilla used them
tropic anchor
When you push and then immediately after realised you used "rejects" instead of "compile errors"
red jungle
π
ripe drum
neo is too much effort to port, I say we call it here
project is abandoned, everyone can go home
red jungle
crimson gull
project is abandoned, everyone can go home
dapper pendant
You know what would be a neat feature? Client to client custom payloads. Like mod-level whisper
inland vaultBOT
New version detected: 1.21.2-pre1.
New version detected: 1.21.2-pre1.
New version detected: 1.21.2-pre1.
crimson gull
Yup
toxic river
nice
prisma pawn
here we go
red jungle
this is way too early
drowsy python
What???
jaunty zodiac
drowsy python
This is highly irregular!!
It is outrageous
prisma pawn
prerelease jumpscare
sand raven
Time for me to invest in modding.
clear urchin
I should have placed my bet for today
@blazing steppe I blame you
you are in Stockholm today
did you ask Mojang nicely??
obtuse marlin
haha xD
inland vaultBOT
frigid flume
well, they did say the release cycle were going to be faster
dapper pendant
Lol when I got a ping here I thought it was a reply to my idea. Last thing I expected was a snapshot
ripe drum
oh god
cosmic acorn
so, in conclusion
they have the same dev cycle
they just named the preview updates "drops"
prisma pawn
finally, data-driven pumpkin overlay
dapper pendant
Well the huge releases are no longer a thing I don't think
drowsy python
Won't someone think of the children modders!?!?
azure bane
Interesting change (empty air bubbles)
lone warren
they have the same dev cycle
what did you expect to change? just curious
dapper pendant
So Idk when they plan to bump the 1.21 -> 1.22 number
red jungle
most of the new stuff is experimental
cosmic acorn
what did you expect to change? just curious
I was expecting them to have 3 updates a year, like 1.22, 1.23, and 1.24 kinda versions, each one much smaller, without a big "content bump" out of experimental
lone warren
well yeah the creaking drop is a winter drop, this is for the bundles of bravery, aka just bundles
ah as in they don't actually put the drops out of eexperimental until the next big update?
lone warren
yeah no that probably would've been better, tbh imo they should've made the drops not have deadlines too. imagine they were free to work on many drops simultaneously and they just drop the one that's done and complete with player feedback into the current update
cosmic acorn
I presume the "deadline" for this one is halloween
lone warren
but now they made it so that this creaking thing is the winter drop specifically, and there will likely not be much changes into it from what we see now
lone warren
I presume the "deadline" for this one is halloween
if it was halloween they would've not shut up about that, def not
that's why they named it winter drop and avoided any talk about halloween
lone warren
I assume there will be something small on the day of halloween though
cosmic acorn
but at least internally
lone warren
but yeah I don't expect the creaking before mid nov, hopefully I'm wrong but also hopefully I'm right because the later it is the more they can change
lone warren
it may not be officially a halloween update
might be one of those "we hope but we won't tell you that we hope" situations
true grotto
There's a typo in the blog post.
haughty raven
my weak ass eyes are struggling to see that typo
i mean i know theres 3 ps but my eyes are only seeing 2 
blazing steppe
I should have placed my bet for today
<@213343613583622144> I blame you
you are ...
Still on my way, gonna complain in person tomorrow
dapper pendant
Yo recipe book categories are a registry now
drowsy python
Sooo what have they done?
dapper pendant
New StreamCodec.composite for 8 fields
haughty raven
red jungle
ah thanks I was also refreshing the page π
there is no a general context thing
might be usable for modeldata π
haughty raven
ah thanks I was also refreshing the page π
refreshing? ive been sat watching the git workflow xD
just saw "pushing remaining versions to remote" logged
which meant its now on git yay \0/
spiral tapir
π
red jungle
there also seems to be some ingredient remainder available now
what makes me sad however is that the ingredient STREAM_CODEC has not been deleted yet π
spiral tapir
gotta make sure it works
clear urchin
you know
sometimes you need to force a release after release after release
what if GC leaves garbage
dapper pendant
Tbf it does feel like multiple System.gc calls work better than one
And even when it doesn't, the second and third calls don't then do much at all and are very fast
clear urchin
but kind of sad they are not logging errors
dapper pendant
This is mid-crash
System.gc doesn't even throw anything (or it's not supposed to anyway)
It's just an extra protection so the game doesn't crash while crashing
sand raven
modding just have been invested in
lucid totem
the Z translation offset changed from -11000.0f to -10000.0f at the top of the GameRenderer diff
round perch
Yeah, I also just noticed that. The "missing" 1000 are then offset on the PoseStack stored in GuiGraphics
grand crypt
modding just have been invested in
why's that?
split wharf
<@93900229476089856> happy? π
still need recipe id, but getting happier
lucid totem
~~might be usable for modeldata~~ π
looks overengineered for that purpose
dapper pendant
I haven't checked if that is available or not
It is not
dapper pendant
Lol this is a funny fix
- "armor_trimmed_ResourceKey[minecraft:recipe / minecraft:ward_armor_trim_smithing_template_smithing_trim]": {
+ "armor_trimmed_minecraft:ward_armor_trim_smithing_template_smithing_trim": {
My confusion as increased
Why is it an ExtendedRecipeBookCategory
It seems to separate the search categories from the basic, but the one interface was good enough imo
And RecipeBookCategories is back
red jungle
It seems to separate the search categories from the basic, but the one interface...
they split them on purpose to make the meaning clear, I think
conceptually BasicRecipeBookCategory got moved to RecipeBookCategory and RecipeBookCategory to ExtendedRecipeBookCategory
drifting ice
I understand, but agh
Also EquipmentLayerRenderer will need to be patched to add back in the ResourceLocation -> RenderType function since they hardcoded it again
Recipe book categories are a registry though
ancient frost
ARGB fields can always be encoded either as a vector of floats (order: [R,G,B,A] or as a single packed integer (order: ARGB)
timber jolt
iirc there's a color codec
spiral tapir
> ARGB fields can always be encoded either as a vector of floats (order: [R,G,B,...
yeah...
red jungle
I hope that RecipeSerializer#streamCodec() will disappear in the next couple of days
if that happens, I suppose that RecipeSerializer could even be removed, and replaced by a Codec 
drifting ice
Update time! https://github.com/ChampionAsh5357/neoforged-github/commit/555e0b41fd8846cf26b409b4ee0e040787b93f07
Basically a few recipe book changes and context params being renamed for general use
round perch
I've played around a bit more with the effect curing rejects and I can't think of a way to make the existing EffectCures system work without running into vanilla network compatibility issues, particular with respect to the creative inventory being client-authoritative:
- Switching honey to use
ClearAllStatusEffectsConsumeEffectwith the appropriateEffectCureand taking a honey bottle out of the creative inventory on a NF client will cause a vanilla server to receive an unconditionalClearAllStatusEffectsConsumeEffect, making it effectively equivalent to milk - Having any item with a
ClearAllStatusEffectsConsumeEffectwith anEffectCurespecified in the inventory on a NF server with a vanilla client, switching to creative mode and interacting with the slot holding that item will cause the NF server to receive an unconditionalClearAllStatusEffectsConsumeEffect, making it effectively equivalent to milk
The better solution in my opinion would be to switch all vanilla cases to RemoveStatusEffectsConsumeEffect and then replace the EffectCures with tags (which appears feasible at first since RemoveStatusEffectsConsumeEffect takes a HolderSet). The milk and totem cases could use a blacklist tag with Neo's NotHolderSet (which would automatically be vanilla network-compatible due to custom holder sets being synced as a resolved list of holders across a non-Neo connection) and the honey case would use a new custom holder set that syncs as a tag key across Neo connections and as a resolved list across non-Neo connections (the vanilla named holder set wouldn't work properly with a Neo client on a vanilla server since the tag would get synced as a tag key to the server, which wouldn't know that tag).
The problem with this idea is that you cannot get a HolderSet.Named from a registry at the time where these consume effects are constructed.
cc @tropic anchor since you were looking at this as well
red jungle
I'll quickly port Kits to 1.21.2-pre1; then leave for the evening π
round perch
π
split wharf
I hope that `RecipeSerializer#streamCodec()` will disappear in the next couple o...
please no
drifting ice
I'm pretty sure it's just going to be replaced with the slot display and property set system, which makes sense
Though, since most of the data needs to be synchronized anyway, it seems like just a different stream codec to send
split wharf
it doesn't contain an id
dapper pendant
It doesn't yet
split wharf
mojang has no incentive to sync an id
dapper pendant
I hope it will before streamCodec goes away, and I hope that's soon
split wharf
they don't need persistant info on the client, all they need is an integer id that matches the server for communication purposes
which is what they currently have and will likely continue using
red jungle
The serializer doesn't contain the id either?
split wharf
it's held by the recipeholder
which is a recipe, id pair with a stream codec
and what was previously synced
red jungle
Yes but it isn't anymore
red jungle
It would be possible to sync id + recipe display if you wanted to
But syncing the recipe sounds wrong
dapper pendant
Is the ID needed for anything other than favoriting and debugging?
split wharf
all recipe serialization (which includes favoriting), debugging and display to users, all customization resource formats that interact with recipes
recipe sorting, plugin and kubejs based recipe removal
for serialization obviously favoriting, screenshotting, future recipe tree persistence, crafting history
dapper pendant
If Mojang makes advanced tooltips show recipe ID, then they'll need to sync the ID
Not saying they have this idea, but it's worth trying to suggest it
split wharf
I was told by boq that "none of this stuff should be run on the client" and they don't seem particularly keen to be syncing it
third party mods additionally interact with recipe ids, ae2 for instance needs it for pattern filling
split wharf
for these specific problems yes
EMI runs into other problems, off the top of my head getting remainders from recipes
red jungle
There were some changes to remainders and slot displays
There is a remainder display now or sth like that
split wharf
don't see that
spiral tapir
don't see that
SlotDisplay$WithRemainder
split wharf
well now isn't that cute
okay yeah I only care about recipe ids viva 1.21.2
the concept of an entire slot display for remainders is kinda scary because what does a variable remainder mean but w/e it's probably fine
now just to hope every modder actually implements this correctly on their recipes. which they'll do, right?
scarlet pawn
wdym by variable remainder?
split wharf
what does [oak_log, iron_ingot, grass_block] remainder: [oak_plank, slime_ball] mean
you just kinda gotta hope that modders are nice
also technically the current implementation erases remainder information for tags
dapper pendant
what does [oak_log, iron_ingot, grass_block] remainder: [oak_plank, slime_ball] ...
I would say that would mean the remainder is derived from some other factor
jaunty zodiac
There's a typo in the blog post. <:thinkies:932605814995292192>
No I wasn't in the post (jk)
Don't show that to PhoenixSC kekw
red jungle
I'll look at fixing the recipe book test; might not finish today π
round perch
I'm gonna delete the FoodProperties reject without re-implementation for now. The associated functionality is now handled by ApplyStatusEffectsConsumeEffect and MobEffectInstance is constructed with a Holder<MobEffect>, meaning registry order between MobEffect and whatever uses the MobEffectInstance shouldn't be relevant
round perch
I'm probably late with this realization, but I just noticed that the client only ever has the RecipeDisplays for unlocked, non-special recipes
split wharf
so the options are
- neo handles the syncing with additional data and everyone is happy
- every mod with recipe viewer compat needs to run their own networking with proper packet ordering to have usable data at reload time with potential race conditions
- recipe viewer mods are always required on server and all have their own networking layer that mods need to implement
red jungle
I think we'll see once the dust settles π
round perch
Small note on the ResourceLocationArgument reject: the functionality has been moved to ResourceKeyArgument and I've added a command exception when recipe lookup is done in a client command. Advancement lookup in client commands should work like it did previously
We're down to 6 rejects and the RecipeBookCategories patch which may need to be moved to SearchRecipeBookCategory
red jungle
Recipe book is mine
round perch
Fine by me, I'm not touching that with a ten foot pole
round perch
What do we want to do with RecipesUpdatedEvent (fired client-only when recipes are received)? Putting it back where it previously fired seems useless because the data received in the ClientboundUpdateRecipesPacket is neither useful to react to nor useful to cache as its trivially available at any later point. Firing it from the ClientboundRecipeBookAddPacket handler when ClientboundRecipeBookAddPacket#replace is true doesn't seem useful either since it never contains all recipes from the server as noted earlier.
The FuelValues part of the ClientPacketListener reject should still be fine in the ClientboundUpdateRecipesPacket handler since it's completely unaffected by the recipe changes.
lucid totem
What do we want to do with `RecipesUpdatedEvent` (fired client-only when recipes...
remove for now imo
until we make a decision on what to do about recipe syncing
round perch
Probably best, yeah. TagsUpdatedEvent also fires right before ClientboundUpdateRecipesPacket is received (at least for reloads), so there is still another event with almost exactly the same timing
red jungle
since when is there a regular Codec for RecipeDisplays?
round perch
Since their inception π
ripe drum
hmmm, i have some time on my hands fetches kits
round perch
You won't find much left except the recipe book stuff, an enderman reject which I'm currently dealing with, an ingredient reject and a rendering reject
ripe drum
hmm, no comp errors?
round perch
Might be some left in the test sourceset
ripe drum
that's good if so 
timber jolt
hmmm, i have some time on my hands *fetches kits*
maty again stealing children
ripe drum
yes
red jungle
maty: don't touch the recipe book π
ripe drum
i'll leave all the touching to you
red jungle
they haven't fully dehardcoded the recipe book but good progress has been made
red jungle
~50 compile errors left
lucid totem
i have a message from mojira announcements about pre2?
hoary cipher
red jungle
inland vaultBOT
New version detected: 1.21.2-pre2.
red jungle
I think we're in good position to finish 1.21.2-pre1 but we'll see once the diff is available
tidal siren
The fact that we got a pre2 already is concerning
lucid totem
three notifications?
ancient frost
"we've replaced the recipe book with a recipedia!"
tidal siren
three notifications?
Yeah the automation is fucked
ripe drum
three notifications?
yesβ’οΈ because the n8n script has personality issues
lucid totem
I feel like I've seen n8n brought up many times before
tidal siren
It also breaks Snowman so it takes 40 minutes to resetup the repo
inland vaultBOT
ripe drum
I feel like I've seen n8n brought up many times before
well no one looked at fixing it so like
lucid totem
ah
ripe drum
it's fineβ’οΈ it's not mission critical
lunar dagger
alright, which one of you forgot to ping <@&1067092163520909374>
ripe drum
you
lunar dagger
tidal siren
seems like there's a community throttle node https://github.com/umanamente/n8n-nodes-throttle/tree/master
lucid totem
@round perch that offset change from last pre was reverted 
round perch
tidal siren
in 110 of 281
ripe drum
huh this is weird, the workflow isn't running only every 45 seconds
blazing steppe
Yo problem causers, I'm gonna drop by the office in Stockholm tomorrow
ripe drum
ok i might have fixed it by having it always run at fixed 60 second intervals always at 00
we shall see in a couple of daysβ’οΈ
lunar dagger
huh this is weird, the workflow isn't running only every 45 seconds <:thinkies:9...
what was it running at
ripe drum
at 00 and 45
for some reason
although it was set to run at 45 second intervals
and 60-45=15 last time i checked
sand raven
Yo problem causers, I'm gonna drop by the office in Stockholm tomorrow
watch out for the machine gun security guy
lucid totem
Blocks.rebuildCache appears to finally be gone
(this probably happened before pre2, I just only got around to noticing it now)
red jungle
Doesn't look like we are impacted
round perch
Yeah, it looks pretty harmless
scarlet pawn
... what does that mean?
red jungle
Can someone take care of the remaining test compile fixes? π
The pins are just my personal todo list of things to check π
scarlet pawn
lol
drifting ice
Primer's updated: https://github.com/ChampionAsh5357/neoforged-github/commit/a31ccccbfdf38a29ae5782badd3d8878e25c7919
Main change is the SoundEvent record and add a generic ProjectionType to combine the vertex sorting and layer ordering logic
round perch
I've got all test compiling and I'm now fixing the block/item ID nonsense
round perch
Yeah, already done that
red jungle
Cool, thank you XFact!
red jungle
Remember to push at some point π
round perch
Yeah, I'm almost done, I just got rekt by data in the tests
round perch
There we go. I got into a world but got spammed with GL errors and crashed during gametest
red jungle
progress
round perch
Ok, the GL spam comes from the PostChain reject. One of the test mods enables the stencil on the main render target and since I instinctively always enable Fabulous mode, I of course have a post-processing shader running at all times, which then causes a depth mismatch between the different render targets involved in that post-processing shader
red jungle
not sure I understand
does stencil add 8 bits of depth?
testmod datagen is up to date 
that's suspicious
red jungle
oh
round perch
does stencil add 8 bits of depth?
As far as I understand, the stencil bits are part of the depth buffer to some degree
red jungle
in practice yeah
a depth mismatch between the different render targets involved in that post-processing shader
where does that crash then? opengl is not happy?
round perch
The crash and the GL spam are separate issues
round perch
I've narrowed down the crash to CommonHooks.onChunkUnload() where we do the POI memory leak cleanup. For some reason that causes the chunk to load again and immediately crash when checking the data consistency:
Caused by: java.lang.IllegalStateException
at net.minecraft.world.level.chunk.storage.SectionStorage.getOrLoad(SectionStorage.java:136)
at net.minecraft.world.entity.ai.village.poi.PoiManager.checkConsistencyWithBlocks(PoiManager.java:232)
at net.minecraft.world.level.chunk.storage.SerializableChunkData.read(SerializableChunkData.java:308)
at net.minecraft.server.level.ChunkMap.lambda$scheduleChunkLoad$17(ChunkMap.java:568)
at java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:646)
If I comment out the call to the hooks method, then it works fine. To reproduce this, place any block with a POI (I used a bed) and then fly away from it far enough to unload the chunk
round perch
I've dug into the PostChain reject some more and I just can't find a way to make the forwarding of the stencil flag work again
drifting ice
The stencil buffer was originally applied in the RenderTarget, correct?
Have you tried looking at allocating it during the RenderTargetDescriptor? I think that's the closest replacement to the post chain logic
round perch
The stencil buffer was originally applied in the `RenderTarget`, correct?
RenderTarget is where the flag is kept and applied. The problem I'm facing is applying that flag to all intermediate targets of a PostChain (which still exists) if the final output RenderTarget has the stencil enabled (which is what was done in the snippet that's left in the reject)
round perch
Have you tried looking at allocating it during the `RenderTargetDescriptor`? I t...
I have not, I'll have to see whether it's an option to use that
scarlet pawn
are you still in KITS or back in the main one?
Answered my own question. Nevermind.
This is to prevent the default cause where graphics cards do not support stencil bits.
... Is this even realistic anymore?
Yeah. OpenGL 3.3 requires DEPTH24_STENCIL8
And why do we even have the option useCombinedDepthStencilAttachment to use separate attachments? The docs for the config don't really say why that would be good
Is that option just ancient? OGL 3.3 requires GL_DEPTH_STENCIL_ATTACHMENT to be present, so I'd say that can be yeeted (but not now)
scarlet pawn
`RenderTarget` is where the flag is kept and applied. The problem I'm facing is ...
I am trying to get into this as well, why do you want to forward it?
As far as my understanding goes, from a mod perspective, you'd only need it on targets that are attached when your custom render targets actually render.
As far as I understandt the render passes, they don't actually allow multi-pass rendering, right?
Hm okay no, it does have that
scarlet pawn
What I experimented with is no longer enabling the stencil buffer imperatively (I think this comes too late)
I used a FML feature flag instead, but we'll run into the problem of when we actually declare it
MainTarget is created before ClientModLoader even gets a chance to do something
scarlet pawn
Alright, @round perch This is more involved than just restoring the patch for sure.
Wdyt about this?
https://github.com/neoforged/Kits/pull/6/files
One annoying aspect is that we do not have access to the NF config-file at that point in time
inland vaultBOT
New version detected: 1.21.2-pre3.
red jungle
<@&1067092163520909374>
drowsy python
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAa
inland vaultBOT
New version detected: 1.21.2-pre3.
blazing steppe
It wasn't me I swear
drowsy python
What?? On a friday?
what are they doing?
clear urchin
Schurli is in the office, duh
drowsy python
It wasn't me I swear
You should have distracted them more!!
blazing steppe
Schurli is in the office, duh
I left the office over an hour ago
blazing steppe
You should have distracted them more!!
They didn't let me past the lobby
jaunty zodiac
quick link for ya https://www.minecraft.net/en-us/article/minecraft-1-21-2-pre-release-3
drowsy python
this is a small one
hopefully snowman is small as well
drowsy python
if so I predict RC on Monday and release on next Monday
subtle leaf
another one already?
hallow basin
if so I predict RC on Monday and release on next Monday
f-you, Saturday release
oops sorry, didnt mean to ping
prisma pawn
again?!
clear urchin
(my main question is, when is the betting machine done /s)
ripe drum
I predict next year
inland vaultBOT
prisma pawn
(my main question is, when is the betting machine done /s)
site should be back up in a day or two actually
tidal siren
I screm in two Actions being triggered
hallow basin
this is github actions, right?
round perch
Alright, <@165621075642679297> This is more involved than just restoring the pat...
Looks like a reasonable solution at a glance. Have you tried it with some of the post-processing effects (Fabulous, spectral arrow, spectating a spider) to make sure it doesn't spam GL errors)?
scarlet pawn
I went into fabulous and yes it fixes the GL error spam
I also think we should really somehow enable synchronous GL debug callbacks (as an option?)
it makes debugging where the errors happen feasible while the asynchronous ones don't
round perch
Good. I'll take another look when I'm at a PC
scarlet pawn
I am debating whether we should just rip out stencil in the port and readd it via a PR afterwards, since it does require a functional change
(the change being that mods have to declare they need the stencil buffer in their mods.toml rather than programatically)
round perch
Considering your proposed change is basically already ripping it out entirely, that sounds reasonable to me
scarlet pawn
So we just ignore the reject and restore the system after the squash?
just asking whether we'll also leave in the non-functional stencil code for now
round perch
Rip out the entire stencil code, comment out the test mod and then re-implement it with a PR later
scarlet pawn
I don't think the test mod has an assert anyway
Ah yeah ofcourse, enableStencil won't work either π
I'll get on th at
scarlet pawn
Well it's less about the review and more about being able to trace it back to some PR description later
I hate the larger changes that just "appear" in some squash titled "Port to 1.2.3"
red jungle
True, I suppose it's quite an obscure feature
lucid totem
How often is onBake called in our registry callbacks?
Is it every time we sync registries?
scarlet pawn
You need to configure your IDE to not use wildcard imports @round perch π
I'll fix the formatting and wildcard imports on the Kits branch
round perch
You need to configure your IDE to not use wildcard imports <@165621075642679297>...
Never (at least not globally)
I try my best to un-wildcard them immediately but sometimes I miss them
scarlet pawn
Your IDE will just auto-insert those if you don't disable it at least for that project
Just store project-specific settings
round perch
Your IDE will just auto-insert those if you don't disable it at least for that p...
I know, but there's a quick-fix option to un-wildcard imports.
Project-specific settings are only really useful if a project actually stays around π
round perch
Don't even need to optimize imports, just adding another import from a package that already has enough imports to reach the "switch to wildcard" limit does it as well
red jungle
Just set the limit to 999
scarlet pawn
I can see the point that for Kits this is annoying
since it's used rarely and is not shared with the normal NF settings
We could just check that file into the repo btw
In NF itself
That file being the project-specific override for that wildcard import setting
round perch
Kits is actually the project that has survived the longest (except this time because I made a second clone to selectively copy over the changes I had done for shits and giggles on the first 1.21.2 snapshot that has a Kits branch). For PRs I always clone fresh for every branch and delete the clone when the PR is done.
ripe drum
that sounds painfully slow
spiral tapir
I've narrowed down the crash to `CommonHooks.onChunkUnload()` where we do the PO...
#789950127774105602 message last one could be related
round perch
Could be, yeah. We've got a fix that should be more stable than hoping loading and saving doesn't get in each other's way though
round perch
that sounds painfully slow
Meh, not really, cloning is quick and setup is done in about a minute (which I would need to run anyway to properly switch between branches on a single clone)
Btw, looking at the stencil PR more closely: LevelRenderer is not the only place where a RenderTargetDescriptor is created, PostChain#addToFrame() does as well and the entity outline render target is explicitly created in LevelRenderer. This means both internal targets of PostChains and the entity outline target ignore the stencil setting. It's not clear to me why that doesn't spew GL errors though (unless going from a buffer with stencil enabled to one without it is somehow fine and the other way round isn't)
scarlet pawn
Kits is actually the project that has survived the longest (except this time bec...
.... what π
scarlet pawn
Btw, looking at the stencil PR more closely: `LevelRenderer` is not the only pla...
Probably because no entity outlines were visible. Good catch, we should check those places too via call hierarchy
But I'd also say: not every RTT needs to have depth+stencil
We need to check in which cases it uses a direct FB copy
round perch
Probably because no entity outlines were visible. Good catch, we should check th...
I tested both entity outlines and the spider spectator shader, neither caused GL errors
scarlet pawn
I saw a "use_depth_buffer" (or similar) in the post processing chain definition that we have to look at
In that case it was likely not using a FB copy
Yeah so com.mojang.blaze3d.pipeline.RenderTarget#copyDepthFrom is the method to look out for
Well that and any render target that mods might render directly into
ripe drum
Meh, not really, cloning is quick and setup is done in about a minute (which I w...
I suppose you have a faster machine
round perch
Well that and any render target that mods might render directly into
RenderTarget#copyDepthFrom() is used for the item entity, translucent and particle targets, all copying from the main target. The entity outline target is one mods will also render into, either automatically by virtue of standard entity renderers or manually for certain tricks
scarlet pawn
Yes, I think I only handled the main target and the ones that do copyDepthFrom correctly, I'd still have to check which ones get rendered into potentially
round perch
The entity outline case is trivial to handle, I just tested that
scarlet pawn
Okay this is a hunch and not confirmed: the intermediate targets used within the post-processing chain itself likely only blit and don't need stencil
except for:
- they declare that they want the depth too (I think that's what that
use_depth_...flag does) - the main RT since that is obviously rendered to i.e. when no PostProc chain is used
- anything else that level geometry is rendered into
round perch
Okay this is a hunch and not confirmed: the intermediate targets used within the...
Basically, yes. Every PostPass sets up its source and target buffer and the shader and uniforms and then draws a single quad and uploads it
round perch
except for:
- they declare that they want the depth too (I *think* that's what t...
This is what the use_depth_buffer flag controls: p_366564_.bindSampler(this.samplerName + "Sampler", this.depthBuffer ? rendertarget.getDepthTextureId() : rendertarget.getColorTextureId());
scarlet pawn
Yes that's what I thought, but that just blits and doesn't do a FB copy
We need to be really sure that doesn't screw up when it blits btw
the texture is usually a combined DEPTH_STENCIL so what do you actually get when you blit from that
round perch
That is a good question, I have absolutely no clue π
scarlet pawn
Oh we probably have to account for that!
Okay or maybe not since the default value for this texture parameter seems to be to return depth
GL_DEPTH_STENCIL_TEXTURE_MODE
Specifies the mode used to read from depth-stencil format textures. params must be one of GL_DEPTH_COMPONENT or GL_STENCIL_INDEX. If the depth stencil mode is GL_DEPTH_COMPONENT, then reads from depth-stencil format textures will return the depth component of the texel in Rt
and the stencil component will be discarded. If the depth stencil mode is GL_STENCIL_INDEX then the stencil component is returned in Rt and the depth component is discarded. The initial value is GL_DEPTH_COMPONENT.
And apparently it's only been available since GL 4.3
red jungle
weird that part of the suggestions are cut on the left
java.util.ConcurrentModificationException: null
at java.base/java.util.LinkedHashMap$LinkedHashIterator.nextNode(LinkedHashMap.java:1023) ~[?:?] {}
at java.base/java.util.LinkedHashMap$LinkedKeyIterator.next(LinkedHashMap.java:1046) ~[?:?] {}
at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133) ~[?:?] {}
at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939) ~[?:?] {}
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?] {}
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[?:?] {}
at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) ~[?:?] {}
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?] {}
at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682) ~[?:?] {}
at TRANSFORMER/[email protected]/net.neoforged.testframework.client.TestsOverlay.render(TestsOverlay.java:54) ~[%23221!/:?] {re:classloading}
at TRANSFORMER/[email protected]/net.neoforged.neoforge.client.gui.GuiLayerManager.renderInner(GuiLayerManager.java:69) ~[%23217!/:?] {re:classloading}
at TRANSFORMER/[email protected]/net.neoforged.neoforge.client.gui.GuiLayerManager.render(GuiLayerManager.java:59) ~[%23217!/:?] {re:classloading}
at TRANSFORMER/[email protected]/net.minecraft.client.gui.Gui.render(Gui.java:250) ~[%23216!/:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
at TRANSFORMER/[email protected]/net.minecraft.client.renderer.GameRenderer.render(GameRenderer.java:500) ~[%23216!/:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
weird crash
lucid totem
scarlet pawn
Any if I commit the result of applyAllFormatting and fixing the wildcard imports?
It runs until it tries to fix patches which obviously fails, but that's not relevant now
scarlet pawn
Eh, not wanna create needless conflicts since this is automated π
but it's 95% import reordering anyway
Since we're at pre2 already we should probably be moving back to the main repo?
red jungle
I think it's a bit early
at least go through the todos π
at the very least add all the missing ATs, idk
it would be good to get testing ASAP
(and bug fixes)
ripe drum
I won't give release more than a week
scarlet pawn
For any non-trivial system reworks I'd rather have history, tbh
red jungle
we should:
- convert patches to ATs (most should match
TODO .* AT) - fix missing rejects
at the very least
I'm going through the recipe book again as usual π
drifting ice
Ok, only interesting change this pre-release is the unsaved listener as the rest were bug fixes
red jungle
to fix the remaining rejects (more like: delete them)
scarlet pawn
to fix the remaining rejects (more like: delete them)
Hmhmhmhm I suppose I am inconsistent on that. If the stencil stuff had happened earlier or the cycle was longer we'd fix it in KITS i suppose
I'll take a look at the ATs
TODO.*AT also matches this one π
scarlet pawn
It's certainly easier than manually getting the original visibility π
ripe drum
it's not like patch gen affects mojmap ports
red jungle
OK I still didn't manage to fix the recipe book test
(or maybe it works now but I have to go)
there are a couple of related // TODO 1.21.2: make this accessible to mods.
I guess just push stuff once you can and we'll crowdsources some of the fixes π
scarlet pawn
The stupid parts are great but having to on-the-spot rearchitect some system often leads to subpar results
random tangle
I wonder how hard it will be to port my mod to 1.21.2.
scarlet pawn
as always: it depends
scarlet pawn
Alright, as discussed I removed the stencil flags for now with the intent to re-implement it later.
round perch
π
I've got three things on my todo for re-implementation once we're on the main repo:
- Effect cures (prototype sitting in a stash)
RenderTooltipEvent.Color: could be implemented as aRenderTooltipEvent.Texturesevent if considered useful since tooltips are now drawn with textures instead of color-only boxesLevelRenderer#requestOutlineEffect(): whether the outline effect shader is hooked up is now determined when the frame graph is built. My idea is to fire aBuildFrameGraphEvent(potentially with aPrephase right before any passes are added and aPostphase right before the graph is built) that allows requesting the outline effect to be enabled.
scarlet pawn
Should we open issues for all of these?
round perch
Pinning them is probably sufficient
tropic anchor
IngredientTests.testComponentIngredient with strict set to false fails due to the ingredient checking for the default item_name predicate for the diamond axe
red jungle
OK, fixed it by using a different component π
ripe drum
@scarlet pawn did you start ats or shall I
ripe drum
well we can do patches now
scarlet pawn
Yes
ripe drum
unless someone has some reason not to
scarlet pawn
Tech? Idk
ripe drum
we kinda should start patches because patch review
@red jungle do you have any objections to patch gen
or @round perch
round perch
Fine be me
red jungle
No objections from me
You can also add ATs, squash and push to the main repo if you want
Patch review? Idk what you're talking about π
ripe drum
looking over the patch diff and seeing if anything is suspicious
scarlet pawn
I'll do the stencil stuff PR once it's in the main repo. One open question is: do we move mod loading to start earlier or do we introduce declarative features that Neo can declare and mods can enable via mods.toml? Think milk fluid or stencil buffer
ripe drum
wasn't that discussed before 
scarlet pawn
I don't remember
This is not per-world feature flags obviously. Personally I favor fabrics approach of running ctors before Minecraft.<init> is called but it conflicts with running mod ctors concurrently with resource reload. I don't remember if we wait for ctors to complete before actually starting resource reload or not
ripe drum
we kinda have the features thing in the mods.toml but that's different
and if we were to introduce flags that'd be confusing
scarlet pawn
I'd make that extensible
round perch
This is not per-world feature flags obviously. Personally I favor fabrics approa...
ClientModLoader.begin() is called right after the resource manager is constructed and is blocking, only the later stage (which includes running common and sided setup events) is concurrent with resource reload since most of it runs from the prepare phase of a reload listener
scarlet pawn
@ripe drum how's it going?
ripe drum
i didn't start it, got distracted
scarlet pawn
hehehehe
red jungle
Tsss
scarlet pawn
I can do uit
*it
So just to be clear, I'll do the full squash and move to main repo?
red jungle
Yes
scarlet pawn
And we clean up the remaining rejects in the main repo too?
scarlet pawn
I'll do a quick compare of the patches against main
Oh fun we have a rule to prevent ATs via patches π
Hmmmmm
That's a formatting change innit
round perch
Yeah, that's just IDEA being funny
red jungle
For some reason VF uses a 4 space indent and we use 8
round perch
There are still some patches using 3 spaces π
lucid totem
Btw, should we make it a priority this release to go through and clean up the TODO 1.xy comments once and for all
scarlet pawn
Ah okay, since that's a new method that doesnt really matter I suppose
lucid totem
I don't mind volunteering myself to help with that
round perch
I've been wanting to clean that up as well for a while, never got to it though
scarlet pawn
I'll work on breaking out the ATs from the patches to satisfy the formatter. If anyone cares to scroll through the patch diff: https://github.com/neoforged/Kits/commit/5db35a2de60941a55e0fb1fafc4d8bdaec5a884f
round perch
The diff looks fine at a glance, only MobEffectInstance has a few unnecessary formatting changes remaining from the removal of the effect cure patches
lucid totem
Why have we patched ContextMap?
round perch
That patch was previously in LootParams
It's missing the comment that was present in the old patch
lucid totem
Ok, that's the only comment I had other than yours
Also looks fine to me at a glance
scarlet pawn
"noInterfaceRemoval"
We should really comment in the script why "Removal of interfaces via patches is not allowed!" is a thing
Why was this done?
red jungle
Probably just a misapply
It happened in the past
Imagine that MC removes an interface from a class
It's possible that the fuzzy patcher would add it back
scarlet pawn
Hm it might be an order change in vanilla, actually
round perch
Yeah, for some reason the interfaces extended/implemented by the packet listener interfaces/classes have changed order several times over the snapshots
scarlet pawn
Alright, applyAllFormatting passes
I tried my best to sort the ATs in accesstransformer.cfg
Now for the squash
red jungle
Remember: different branch π
scarlet pawn
yes
round perch
You've got a patch lowering visibility from an AT: https://github.com/neoforged/Kits/commit/5a7657506f123027047849db1a43ad57a632a379#diff-3a804e12ec894d82dcbddaddb57bcd685b09129daec0b6048b1df43e48c1127fL7
red jungle
BTW big PRs will still be squashed unless we decide on a merge freeze until the release
scarlet pawn
Looking into it
Oh I think this was only supposed to be protected and i made it public in the AT
round perch
Yup. Also forgot to remove the TODO comment
scarlet pawn
Good cach, thanks! And fixed
Which pre are we on again? The branch is still named pre1 but we're newer, right?
round perch
I'm pretty sure we're still in pre1
scarlet pawn
Hm no, still pre1 apparently π€
ripe drum
only 53 patches in that diff? that sounds wrong
lucid totem
when are we planning to land custom feature flags
ripe drum
ah wait patches were genned before
lucid totem
I think that's the biggest open feature PR we have rn?
round perch
I think so, yeah
lucid totem
It probably isn't hard to retarget though since it only patches 5 files
round perch
It might even apply cleanly, I don't remember any of the affected files changing
Well, no, nevermind, they added more flags
red jungle
It's a rather low priority PR tbh
It's just that it will have to be merged in 1.21.2 first if we wait
red jungle
ah wait patches were genned before
Yes, a lot of patches were genned before
scarlet pawn
Co-authors list looks ok
Did we have a specific naming scheme for port branches on NF? I don't remember. There was some issue with the Jar compat checker going nuts, right?
Didn't we add an exclude for a specific branch pattern π€
lucid totem
isn't it always port/<version>
round perch
Yup
scarlet pawn
done
Oh right it can't publish the PR until it's rebased blergh
Draft PR for publishing when we do rebase it https://github.com/neoforged/NeoForge/pull/1590
lucid totem
Trivial /s
red jungle
ok rebased
https://github.com/neoforged/NeoForge/commit/b92c510d64ed9506140d74538f4699277e8193bd did not make it over
I added a TODO in the item extension method
looks like some of the parameters are missing (float limbSwing, float limbSwingAmount, float partialTick, float ageInTicks), idk how easy they are to find again
toxic river
?
round perch
looks like some of the parameters are missing (`float limbSwing, float limbSwing...
The entity and armor model changes are a pain in the ass, I'll need to check how to re-integrate that
red jungle
?
yes we know about that
huh
ok I see
need to add -PvalidateAccessTransformers in the setup call; might do it after lunch π
toxic river
also, just for your information, Fast Leaves are broken in the port
sand raven
also, just for your information, Fast Leaves are broken in the port
Are you running 1.21.2 neo locally and trying to tell us the bugs before the port is even finished
Please donβt
Let the porting work be done first, THEN look for bugs
Otherwise you could cause us more work by diverting attention to the wrong areas too early/cause porters to step on each otherβs toes.
scarlet pawn
FYI we moved back to the main repo now
toxic river
Also, I think the bug fix for MC-273464 is no longer useful?
wheat mango
mod logos seem to be broken
almost certainly the custom blit function acting up, I got some results with the default blit
also, is there a reason the MC logo isn't shown in the Minecraft mod? imo it looks nicer and only needs like 3 lines of hardcoding
lucid totem
also, is there a reason the MC logo isn't shown in the Minecraft mod? imo it loo...
make a PR
wheat mango
i will if people actually want it
i assumed there was a reason behind it not being there
tacit arrow
<@84313704874663936> did you start ats or shall I
If we havenβt yet, we should also bump ATs to 11 or whatever so that we can stop shipping antlr or whatever
ripe drum
that can be a non squashed PR
red jungle
grrrr
I guess the events will be downgraded to take HolderLookup.Provider
can we standardize the naming on registries for HolderLookup.Provider?
getLookupProvider is not great π
tacit arrow
I am fine with registries, but if we don't go that route, getHolderLookup is better than getLookupProvider imo
red jungle
it's not a holder lookup though
tacit arrow
ehhh
red jungle
a HolderLookup is a single registry
red jungle
π
haughty raven
if it was simply a HolderLookup then yeah holderLookup would work as that looks up Holders
but its HolderLookup.Provider which looks up HolderLookup.RegistryLookups so registries makes more sense here
round perch
It does: RegistryLookup<T> extends HolderLookup<T>
red jungle
ok but still
tacit arrow
tech's point is it returns a HolderGetter
red jungle
HolderOwner is the one that confuses me
tacit arrow
well
haughty raven
oh yeah it extends my bad
tacit arrow
optionals of a holder getter
but same difference
but it does have a getter to also grab a holder 
haughty raven
but registries still makes more sense imo
since it looks up either a registry or some other lookup to then lookup holders
red jungle
we should rename this
tacit arrow
HolderLookup.Provider is really HolderGetter.Provider
round perch
we should rename this
Add a $ to the name /s
red jungle
tbh I don't care enough
if someone cares make a PR and we'll see π€·
holderOrThrow is a fine name, just not what mojang uses in other places anymore
OK with this last commit the PR should build fine now
FFS now JCC is failing
has JCC even resulted in a net time gain?
I feel like all it does is cause our CI to fail when it's misconfigured
lucid totem
also, just for your information, Fast Leaves are broken in the port
can't reproduce
toxic river
Β―_(γ)_/Β―
oh wait...
ok, it's just my resource pack that breaks the leaves rendering...
scarlet pawn
tz tz tz
wheat mango
it looks like Minecraft-dependencies might have an impossible dependency, causing weird issues
(it has strictly ASM 9.3 but everything else needs 9.7)
scarlet pawn
I think he means the MC gradle dependencies
red jungle
Ah we have it set to strictly 9.3 π€
scarlet pawn
All Minecraft dependencies are set strictly to 9.3, we might have forgotten to actually follow through on "how to override that" π
ripe drum
that's the most important detail
Iove how we managed to do it accidentally in the past but now we're not doing it intentionally when we need to
scarlet pawn
uhm well yes we depend transitively on a dependency that strictly requires asm 9.3 π
scarlet pawn
Can't we downgrade asm? π
You mean upgrade
red jungle
Why are we on 9.7 and mc on 9.3?
We could be on 9.3, no?
But that will probably kill jdk 23 support or whatever
It's better to be able to upgrade it
scarlet pawn
No?
9.3 is a transitive dependency for Minecraft
And I think they only use that for code-generation meaning they don't care about newer JDK support since they never read classes
It's an unfortunate side-effect of a weird-ass JSON library they use transitively now
ripe drum
yes as in we need to upgrade
scarlet pawn
BTW here's the error
Primarily:
Dependency path 'appeng:ae2:19.0.21-alpha.13+optimized-ui' --> 'net.neoforged:neoforge:21.2.0-alpha.1.21.2-pre1.20241013.104426' (modDevApiElements) --> 'net.neoforged:neoform:1.21.2-pre1-20241008.174159' (neoformApiElements) --> 'net.neoforged:minecraft-dependencies:1.21.2-pre1' (clientCompileDependencies) --> 'org.ow2.asm:asm:{strictly 9.3}'
Excluding the transitive dependency seems to be one way of solving this, I'll investigate others
scarlet pawn
This also seems to work when declared in NeoForge, and I like this better (it's explicit)
scarlet pawn
To get the build to pass we need to merge the action change from august: https://github.com/neoforged/NeoForge/pull/1434
I seemingly forgot to actually ready it for review π
red jungle
oh thank god, I was gonna look into that
scarlet pawn
Let's see how it goes
scarlet pawn
The hell
red jungle
ok let's start the debugging fun β’οΈ
scarlet pawn
Well wait
Can we see the webhook event payload in the action log?
It's probably just a dumb mistake on the ref naming
Oh it may be even more stupid. I used a workflow dispatch to run it, but the if checks the pull request webhook event only
red jungle
you pushed the commit and it didn't work
scarlet pawn
No this was manually run: https://github.com/neoforged/NeoForge/actions/runs/11315440248
I am always unclear on which ref the workflow comes from for pull_request events, but I think from the default branch
So the previous build I think used the older workflow
red jungle
- run: echo "${{ github.event.pull_request.head }}"
that should help debug it, no? π
scarlet pawn
In principle, but it won't tell you for the manual dispatch event
We're gonna rebase/squash the port branch again, yes? So I can just push a dummy commit there?
red jungle
yes
toxic river
Can I say what bugs I've found or not?
red jungle
if they are caused by neoforge and not by some resource pack or mod you forgot to disable: yes
red jungle
π
scarlet pawn
If you run manually this happens because that part of the build script also hardcodes the pull_request event
git switch -C pr-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.ref }}
red jungle
well that one might not be fixable
scarlet pawn
Yeah had the same thought
I don't think you can get the PR#
unless you are in the context of a PR event
scarlet pawn
... it is the correct place to fix it
Running JCC makes no sense for a port
Well, I suppose you could try to make it download the previous stable version but good luck π
Oh I know what it is, noob mistake 
red jungle
you can just make it diff the jar against itself as a fix
scarlet pawn
The bug is the ref/heads prefix is superflous
red jungle
if (rootProject.isPreReleaseVersion) {
createCompatJar.configure { it.enabled = false }
checkJarCompatibility.enabled = false
}
this also works π
red jungle
The bug is the ref/heads prefix is superflous
for the ref too?
well yes it is
otherwise the git switch -c would fail
scarlet pawn
yup
red jungle
now rebase again π
idk why we even merged that PR against 1.21.x, it was really pointless
scarlet pawn
Yes stupid mistake
fingers crossed
Okay it fails on game tests now. Interesting. entityFinalizeSpawnEvent isn't that the coremod one?
round perch
Yeah
scarlet pawn
But hey, we still at least got a published PR now
ripe drum
what tests give you maps lol
red jungle
ok the recipe book stuff is finally fixed
round perch
All those tests are set to enabledByDefault. I can't remember why I did that for the map decoration test and I have no idea why it gives you more than one (I've had that happen as well)
scarlet pawn
PlayerLoggedInEvent
Does that fire multiple times perhaps?
In any case it will give you a new one everytime you join the world
red jungle
the problem is the test being enabled by default
round perch
Does that fire multiple times perhaps?
That's the only explanation, but that would be catastrophic.
I always clear my inventory, so it's definitely not just stacking up from repeatedly entering the world
red jungle
they accumulate over multiple logins
round perch
No, I've definitely had multiple added per join
round perch
That was me π
scarlet pawn
I would also have missed that
we still have the todo to auto-generate the method redirection data
red jungle
we'll do it once we move to mdg
scarlet pawn
and whatever would do that should also check that the source method exists with that signature at all
yeah
red jungle
now time for a second pass on Ingredients
I think that we don't need to show a barrier for empty tags anymore
because such ingredients are now always invalid (in vanilla recipes at least)
scarlet pawn
But hey, gametest saved the day!
red jungle
yesss
scarlet pawn
These silent coremod failures are a bit scary
We might also consider crashing in case of the method redirectors if a target class had 0 replacements
round perch
Speaking of coremod failures: Tech, have you had a chance to look at my janky AT PR? π
red jungle
hmm I remember discussing the existence of such a PR
round perch
Hehe, understandable
red jungle
ok let's say we don't need the barriers for empty tags anymore
scarlet pawn
Oh did they fix empty tags in recipes matching empty slots?
round perch
I will as well with my stonecutter-esque crafting system since, surprise, surprise, stonecutter recipes are special-cased in the new sync system for a reason and I will need to do something similar
round perch
How would you go from an Ingredient to a SlotDisplay in the first place though? Ingredient#display() currently doesn't produce usable displays for ingredients where the exact stacks matter
red jungle
I'm adding SlotDisplay ICustomIngredient#display
round perch
π
red jungle
what I am not sure about is the handling of compound ingredients
should these have a custom display implementation that tries to merge the stacks?
round perch
Merge as in remove exact duplicates?
red jungle
well at least it should get all stacks and put them all in a list
oh vanilla already has a composite
round perch
There's a composite slot display
red jungle
nice
god the remainder handling in the displays is not great
the displays for DifferenceIngredient and IntersectionIngredient require custom implementations of SlotDisplay I think
I'm not even sure how though
poor SizedIngredient
it turns out that this Ingredient change kinda sucks
I guess I'll delete this method for now
btw that test fires 3 times apparently
round perch
Yes, which makes no sense to me, that shouldn't be possible in theory
red jungle
https://github.com/neoforged/NeoForge/commit/3e993b1ce4962357368875663eb3fd9b57eb91dd best I could do
max told me he'll take care of porting the fluid ingredients
red jungle
Yes, which makes no sense to me, that shouldn't be possible in theory
the event does fire 3 times
round perch
round perch
How the hell
ripe drum
get in faster
red jungle
looks like tests that are enabled by default have some bugs with their event handling
final sky
i just realised something
actually nvm i forgot streams are constrained to be iterated once
i was about to ask why Composite takes a List and not a Stream in vanilla 
either way, reading git diffs on my phone kinda sucks
round perch
https://github.com/neoforged/NeoForge/commit/3e993b1ce4962357368875663eb3fd9b57e...
Looks fine to me. Just to be pedantic, you have a single unnecessary patch line: https://github.com/neoforged/NeoForge/commit/3e993b1ce4962357368875663eb3fd9b57eb91dd#diff-1e9a481fe10e38e00570eaa6690cc0246a3361655b8362adf776a0faed46e0c7R43
final sky
okay apparently gradle does not want me to work on this today
sigh time to clone neo from github again
A problem was found with the configuration of task ':neoforge:selectRawArtifactNg_dummy_ng.net.minecraft_client_1.21.2-pre1_client-extra' (type 'ArtifactFromOutput').
- In plugin 'net.neoforged.gradle.common.CommonProjectPlugin' type 'net.neoforged.gradle.common.tasks.ArtifactFromOutput' property 'input' specifies file '/Users/max/programming/mc/NeoForge/projects/neoforge/build/jars/extra/client/1.21.2-pre1/client-extra.jar' which doesn't exist.
okay what
ripe drum
final sky
gradle cache nuked
i don't care anymore
it's still there 
okay actually what the hell is going on here
@red jungle hello yes mr api man i can't work like this i'll go back to my thesis for a bit
lunar dagger
"On the Abandonment of Gradle As A Build Tool For Java Projects"
final sky
i WISH
red jungle
Just wait for https://github.com/neoforged/NeoForge/pull/1485
(don't literally wait for it)
final sky
was gonna say ^^;
is there a task i forgot to run by any chance
because i can't really run setup or load the gradle project into idea
red jungle
Nuke your build folders
red jungle
Nuke projects/neoforge/build
final sky
okay is it maybe just idea
because i did that just now
and ./gradlew setup (and neoforge:setup etc.) all work in the terminal
but idea still refuses to import the project because it errors with that same error
yeah no it is just idea what the hell
it is right tho, the jars folder doesn't exist at all in the nf project...
red jungle
Try gradlew :neoforge:runClient
You can add --rerun-tasks once to force the cache to invalidate
random tangle
I have never had as much trouble with a build tool as I have had with Gradle.
red jungle
FYI gradlew genPatches has been back for a couple of weeks
final sky
poor `SizedIngredient`
okay actually i'm not sure how this is a bad thing
since you can still resolve the SlotDisplay to a Stream<ItemStack> just fine
okay so thoughts
i already see a todo on DifferenceIngredient
my question is: why not make a SubtractingSlotDisplay
<T> Stream<T> resolve(ContextMap p_381155_, DisplayContentsFactory<T> p_381077_);
resolve gets us a stream
i guess there is some difficulty in saying "filter elements out of the stream that are contained in another stream"
so that's a fair point
ripe drum
i guess there is some difficulty in saying "filter elements out of the stream th...
eh not really
just turn the stream into a list
or set actually for this matter
round perch
Set won't work because stacks don't implement hashcode/equals
final sky
^
ripe drum
ah right. wrapper object then
lucid totem
Isn't there an ItemStack hashing strategy for fastutil in vanilla by now
round perch
Good point, yeah
round perch
For complex inner ingredients you probably won't get around it, yeah. For simple inner ingredients you have two options:
- Keep using vanilla displays which has the benefit of working for connections to vanilla clients
- Go all the way and disregard vanilla clients entirely when custom ingredients are involved (certainly the simpler option)
ripe drum
oh right, we have to send a vanilla display for difference ingredients
as we are replacing some ingredients in vanilla recipes with diff ingredients
round perch
round perch
Yes
final sky
also resolve() is (unfortunately) generic
so i can't just plop things in a hash set with the item hashing strategy
this is also good tho
because it means we can reuse the same slot display for fluid ingredients as well
should i assume that everything that is not an ItemStack supports proper hashing maybe...?
round perch
FluidStack doesn't support it either
final sky
...oh man
so we can't reliably use a set at all
also am i smoking or is ForRemainders unused in vanilla...?
or rather, not implemented
well... i'm kind of unsure how to do this reliably honestly
so i might just leave this half-broken and move on to more important things ^^;
i can't even make an item specific DifferenceSlotDisplay because i can't just filter by generic type in java 
because sure, i can filter by if the factory is an ItemStackContentsFactory
but that would ignore any custom ItemStack based contents factories other mods might make
terse anvil
i can't even make an item specific DifferenceSlotDisplay because i can't just fi...
Not with that attitude
final sky
okay, genuine idea
DisplayContentsFactory is currently an empty interface
and vanilla only has one concrete Factory implementation
should neo add a factory type method to it?
since in theory, a concrete factory implementation can only ever consume displays of one type, right?
vivid ore
All Minecraft dependencies are set strictly to 9.3, we might have forgotten to a...
Should be able to exclude the bound from minecraft-dependencies within the neo dependency on neoform, I think? Not sure if you already found a solution
scarlet pawn
strict in NeoForge overrides the strict from the transitive dep, even for downstream consumers of NeoForge
https://github.com/neoforged/NeoForge/commit/bfc7e740b784048c2daacf15823cbb94a0ad3a0b
vivid ore
Oh, nice!
final sky
(ping me with updates on ingredient and slot display stuff, i'm going to bed)
drifting ice
Can we have it such that there is a DeferredRegister#register method that takes in a the registry objects resource key and returns the registry object? Thinking about the set id and also the potential naming in sound event
haughty raven
sound events dont have the setId() thing, only items do iirc
but being able to set that id yourself would be nice yeah rather than the register methods auto calling it all the time
drifting ice
Sound event iirc takes in the translation key as a string unless Iβm misremembering how it works nowadays
Yeah, the sound_id, which most people just use the registry name of the sound for
red jungle
A DifferenceSlotDisplay won't work because there is no way of filtering out the stacks
@silent @final sky ^
red jungle
I guess silent doesn't work on my phone anymore
grand crypt
weird
final sky
A DifferenceSlotDisplay won't work because there is no way of filtering out the ...
the issue is we don't know the type of the stacks
i.e. the generic bound
that's why i was thinking about making contents factories typed
red jungle
that's not even the only problem
the other problem is that you really don't want to put the Ingredient inside the SlotDisplay
so you can't do base.stacks().removeIf(s -> subtracted.test(s)) (pseudocode) because you don't have access to subtracted.test inside the display
final sky
you only have access to the SlotDisplay yes
so the best you could do was taking both displays and taking their difference
red jungle
which doesn't really make sense if the ingredients are not simple
hence the use of items() to construct the display π
final sky
the problem is you are discarding information about the components of the item here
items() only generates basic holders
with no data on the item
red jungle
whatever you do, the NBT-awareness of subtracted has to be ignored in the display