1.21.5 Snapshots

[river mural]

but the stream one you could use List.copyOf

instead of going back and forth

also you forgot one copy tech

[floral mesa]

what the-

[river mural]

ListCodec has a copy from a mutable list to an immutable one internally

[gleaming pendant]

[dark wasp]

[gleaming pendant]

wait why would they be different?

[floral mesa]

I mean I doubt there are enough item abilities at once for these copies to be super expensive but it still looks ridiculous

[river mural]

because codecs give immutable lists

stream codecs give mutable ones

GRANTED because you copy it to a set regardless

[gleaming pendant]

everything has to go through the canonical constructor

[river mural]

why do you care that it is immutable

[floral mesa]

java doesn't have a SequencedSet.of immutable collection?

[gleaming pendant]

NO

🤬

[floral mesa]

neither does guava I assume

because it usually feels years out of date

[gleaming pendant]

is there no vanilla component with a set inside

I guess not

[river mural]

public ItemAbilitiesComponent {
    abilities = Collections.unmodifiableSequencedSet(abilities);
}

private ItemAbilitiesComponent(List<ItemAbility> fromCodec) {
    this(new LinkedHashSet<>(fromCodec);
}

[gleaming pendant]

making it unmodifiable in the canonical constructor is not enough

it might still be modified through external means

[river mural]

because you need to copy it

hmph

just make it a SequencedCollection

for what abilities is

[floral mesa]

collection is still modifiable

[river mural]

actually

ignore me

[gleaming pendant]

the thing with the list is that it can make serialization weird

and also stack comparison

unless we force the list to be sorted in the constructor

which we should probably just do

[river mural]

but either way, add an overload that accepts a list of item abilities

[gleaming pendant]

(and at that stage we could just binary search )

[river mural]

and then does the copying

so you don't go from list -> array -> list just to then copy it to a set

also uh

your stream codec

[floral mesa]

what if you just use ImmutableSet

I believe it's guaranteed to keep insertion order

and its copyOf should avoid redundant copies

[gleaming pendant]

this looks reasonable

[river mural]

use .apply(ByteBufCodecs.<ByteBuf, ItemAbility, SequencedSet<ItemAbility>>collection(LinkedHashSet::new)

[floral mesa]

the last constructor does three collection instantiations

because List.of defensively copies the array

at least use Arrays.asList

[sterile abyss]

what gametests fixes, I fixed some stuff and it's pushed. tho we still need to handle the entrypoint

[raven blade]

oh man, we're keeping the item abilities... ah well, maybe someday we won't need em anymore

[gleaming pendant]

[drifting finch]

Did this get rid of the dig?

[river mural]

tech ^ don't bother collecting into a list when you can just collect into a set initially

[gleaming pendant]

yup I saw

[gleaming pendant]

no we should keep it

[drifting finch]

What is the argument for that?
The last I read here from scrolling back up that we want to remove it to be closer to what vanilla does and reduce maintanance

[gleaming pendant]

the ability to define super weird mining rules has already existed for a long time, it's nothing new

[river mural]

also ^ embedded is right, seems that immutable set does keep insertion order

granted then we just have internally it is consistent ordering rather than declared by it being a SequencedSet

[gleaming pendant]

the point of these tool actions is to categorize the tool as being pickaxe-like etc for a variety of purposes

[drifting finch]

As already defined and researched, with the Tool concept any weird mining rule is already possible......

[gleaming pendant]

you misread my sentence

the Tool concept does not change anything, you could always create a weird Item subclass

[drifting finch]

I hate my phone

Actually my point is our *_DIG breaks vanilla compatibility.

Vanilla defines anything pickaxe-like as anything that has a tool rule with mineable/pickaxe

Which could be a any item in the game

Our *_DIG does not match up with that tooling logic

[floral mesa]

we can make the record hold an ImmutableSet instead of a generic Set to make that property clear in the API

[gleaming pendant]

tbh I think sequencedset is not even good enough because there are still multiple ways it can be serialized

it should just be a sorted list

[gleaming pendant]

there is nothing here that "breaks vanilla compatibility" since _DIG purely exists as metadata

(except for SHEARS_DIG which is used by loot tables)

[floral mesa]

there are?

[drifting finch]

Except that this metadata is, per your own argument, used to determine whether something is pickaxe-like, which mojang now defines it self, and which our logic does not follow.

It directly follows that an item with the correct tool configuration for being a pickaxe-like item, is not properly detected by this mechanism and it is not being treated as pickaxe-like, or the other way around.

In other words, a modder now needs to check 2 places whether something is a pickaxe-like, axe-like, shovel-like etc tool

[gleaming pendant]

No. They should only check the tool action

[drifting finch]

No they really should not

[river mural]

isn't the only way in vanilla to check is if it is in a tag

so if something is conditionally a tool...

[drifting finch]

No the only way to check if something is a tool is to check the tool component

Because you can trivially make any item a pickaxe-like tool by adding the tool component with the pickaxe rule

[gleaming pendant]

If you do that you are responsible for also adding the corresponding item ability component

[drifting finch]

No vanilla datapack will do that

[river mural]

but there is no way from the tool component to know which type of tool it is?

[drifting finch]

Because that does not exist anymore.......

It is just a "Tool"

And any given tool can be made to harvest a given target block or not

You can make an axe harvest diorite if you want to

[gleaming pendant]

They can if they use the custom data component. If they don't, they will just be mildly incompatible with some mod recipes or stuff

[drifting finch]

And have it behave like a pickaxe on everything

[drifting finch]

Given that we already discussed that recipes can easily be made to work with TOOL, there is no reason for *_DIG to stay

[river mural]

oh right

we are explicitly talking about the dig actions here

[drifting finch]

It only creates more work on everybody involved

[river mural]

forgot that bit

[gleaming pendant]

Not all recipes in reality

[drifting finch]

Yes it is purely the DIG

[gleaming pendant]

Some recipes accept any pickaxe item

E.g. pickaxe module from modular routers

[drifting finch]

How does modular routers define a "pickaxe"

[gleaming pendant]

ToolAction literally

[drifting finch]

I assume because it mines stone, or diorite or ore

[drifting finch]

Not in code, in context

[river mural]

tbh imo it might make more sense for modular routers to just check for a tool component and then copy said component onto the output

[drifting finch]

The player has no idea that it is represented by a ToolAction

[drifting finch]

That for one too

[river mural]

but yeah, for the dig abilities I don't really have a preference. While mek is in the waifu thing that maty shared, it is literally just so that I can return true for canPerformAction for the atomic disassembler

[drifting finch]

The player thinks that anything that can reasonably mine and drop things like stone, diorite, ores, etc, is a pickaxe-like tool.

That could even be something like a paxel

[gleaming pendant]

The player is in fact using JEI

[drifting finch]

Or a spawned in fence that has the right tool rule

[drifting finch]

That might or might not be the case.... And is at best a weak argument

Not all players play with such tools

And not all datapacks or mod packs require or even include it

Or have all recipes visible at all times

[floral mesa]

also the way JEI works is still a bit up in the air after 1.21.2

[drifting finch]

Additionally, even if you would use _DIG as a component

It can again be added to any item at runtime

[gleaming pendant]

Yes that's the point of these tool actions. They are just metadata for mods that query "is this a pickaxe"

[drifting finch]

Which means JEI is at best a hint it will never be able to display all "pickaxes"

[floral mesa]

as far as I understand, "is this a pickaxe" is now "does the item have a component that allows it to mine mineable_with_pickaxe"

[drifting finch]

Not quite

[gleaming pendant]

More accurately there is no way to check for a pickaxe in code

[drifting finch]

Correct

There is no way to know ahead of time whether something is or isn't a pickaxe

Just whether something can or can't harvest a given block

And take harvest here in the widest possible sense

[gleaming pendant]

All you can check is whether the stack can mine a specific block faster and/or allows drops for a specific block

[floral mesa]

yes that's what I meant, the closest approximation is "does it mine things that pickaxes tend to mine"

[drifting finch]

Correct

You can check it on the prototype default stack, that will give you an estimated array of items that will behave somewhat like a pickaxe

[floral mesa]

So then why do we have two systems for answering that question?

[drifting finch]

So you can prepopulate JEI

[drifting finch]

Well if you scroll back, several maintainers already agreed that we should remove the *_DIG

And talked about it at length

Including with for example Knightminer as the maintainer of Tinker Construct

And researched this topic heavily with Waifu and other tools

To figure out what the best approach would be

And we agreed that we should remove *_DIG as it would make all our lives easier

Less maintanance, more specific, applicable in all situations we found

[gleaming pendant]

No you really shouldn't do that

[drifting finch]

However Tech has decided not to go that route, and leave them in on his own accord.

[gleaming pendant]

In fact you shouldn't check the Tool component at all since we have some methods in IItemExtension that can override it

Better see it as a black box implementation of getMiningSpeed(BlockState) and isCorrectToolForFrops(BlockState)

[drifting finch]

Yeah that could also work

You are correct in that matter

But that does not alter the conceptual idea that we should remove *_DIG

[gleaming pendant]

So how do you even write a pickaxe check?

[drifting finch]

By checking if it harvests the blocks relevant to your context

[gleaming pendant]

(You have these two methods, what do you use as the condition for a pickaxe-like item)

There is not always a context, e.g. in modular routers' case

[drifting finch]

If you want to precheck, you do could do a random assortment of elements from the pickaxe mineable tag

[drifting finch]

MR should just copy the TOOL component from the source pickaxe, or hold on to it

And use that for its "fake" pickaxe when it attempts to mine the block

[gleaming pendant]

Again Tool should not be used direvtly

[drifting finch]

Again I disagree

[gleaming pendant]

Since some tool items do not have that component

[drifting finch]

It is what vanilla uses directly

[gleaming pendant]

We have extension methods

[drifting finch]

It is what vanilla exposes to command blocks, custom functions etc

Given that both of these functions can be expressed in the TOOL component rules

I see actually no reason to keep these methods

[gleaming pendant]

You should never use Tool directly to check for tools since these methods are authoritative

[floral mesa]

but do we need the methods

[drifting finch]

We don't embeddedt

As such they might be authorative right now,

[floral mesa]

you can reimplement them using a custom holderset, no?

[drifting finch]

But that is no argument to keep them

[drifting finch]

Yep, as already talked over with KnightMiner, and tested

[gleaming pendant]

Well, we have them now

KM hasn't even started using components lol

[drifting finch]

?!?!
That is also no reason to keep them

[gleaming pendant]

Indeed not, but once has to provide a coherent design

[drifting finch]

Also we don't have them

You just added them

[gleaming pendant]

Not "oh let's remove this feature and pretend there's a way to approximate it"

[drifting finch]

Which is no argument to keep them

[gleaming pendant]

What are you talking about. I did not add any extension methods

[drifting finch]

https://github.com/neoforged/Kits/blob/25w06a/src/main/java/net/neoforged/neoforge/common/extensions/IItemExtension.java

Those two methods don't exist on the current IItemExtension

Or am I missing something here?

Because I was trying to understand how they are wired up and used

And as far as I can see they don't exist

Did somebdoy remove those already?

[gleaming pendant]

Oh right they're vanilla methods actually

https://github.com/neoforged/Kits/blob/5263102b58ee47da27407d4ea9500d73999b5097/projects/neoforge/src/main/java/net/minecraft/world/item/Item.java#L275

[drifting finch]

Ahhh

Hmm

Okey well you can call those methods then

That is still no argument to keep *_DIG

[gleaming pendant]

Well what do you call them with?

Check that the tool can break stone faster than 1.0?

[drifting finch]

? The tool component..... Like vanilla has it implement

Ahh

What do you compare their results

Sorry the statement "call them with" was a confusing request

That depends, I would say it is a pickaxe if it drops stone, or at least harvests (and with that I mean drop) a reasonable amount of entries within the mineable tag

But that could also depend entirely on the context I am in

[gleaming pendant]

Ok maybe let's ask desht

Hi @tepid wagon we are looking into deleting the DIG tool actions. Is that a problem? You are using it in a Modular Routers recipe

[drifting finch]

I mean based on looking at his recipe, he really only should check for the TOOL tag, and as pup already stated just copy it over into the card, and then use that tool tag to try to harvest the resources

That should then also apply the correct speed and harvestabiltiy rules

[merry falcon]

The ItemAbility was to support nbt/datacomponent changing items right? We are dropping support for those due to vanilla design?

[drifting finch]

You can still do that, all the other places are still there

The ItemAbility check could not be used to consume energy from NBT for example, as it could be called many times a frame/tick to check for harvestability

[river mural]

the only real dig action that might be problematic is the shears one. Unless things changed and we no longer have to override the loot tables for that stuff

[drifting finch]

Unsure

It is the one I researched the least

[gleaming pendant]

We still need the loot tables for that one

[drifting finch]

Okey then we might want to keep that

And at least point that out to some mojangstas that their architecture there for the tools could be improved......

But that should not be a high priority in my opinion

Does kits include the Minecraft resources anywhere?

[proven compass]

this is so cool what

where does this search

[drifting finch]

In WAIFU

[tepid wagon]

It shouldn't be a problem. I can adjust to that, as discussed above

[gleaming pendant]

hmmm

SWORD_SWEEP

FFS vanilla is checking ItemTags.SWORDS for the sweep action

and we patch that to use the SWORD_SWEEP tool action instead

ok easy we'll move that into the default method

changes made, see https://github.com/neoforged/Kits/commit/32a5216f585f80a93e479409d28d02f55e919905

[frank sigil]

there are still compile errors related to the missing annotations

ugh a bit late but why the complicated sorted set and not just our set codec?

[sterile abyss]

replace those with the test framework one

[river mural]

regardless

it is irrelevant

as that isn't the route tech went

[frank sigil]

I has menu

I has test world

there are still some rejects someone didn't remove when fixing / have not been fixed

[frank sigil]

getting into the main menu 3rd try and getting a world generated and entering first try

[fallow merlin]

Poggers

[gleaming pendant]

That looks a bit ugly

Do we not whitelist BlockState already?

Yeah we do

[glass wagon]

Decided to gen patches and see how many rejects there are for 25w08a and so there is a total of 41 reject files and here is 25 of them

[gleaming pendant]

Ah but apparently it's not even necessary to whitelist them anymore?

[glass wagon]

StateHolder overrides equals and hashcode to make them final

[gleaming pendant]

Then there's no need to mark them as valid

So please try to remove the call to mark them as valid, and the special case inside markComponentAsValid

How are we doing wrt the other projects?

[frank sigil]

it's because the whitelisting fails due to StateHolder implementing equals and hashcode as final redirecting to the object implementation

[frank sigil]

does that work for parent classes?

[glass wagon]

yes

[gleaming pendant]

These checks exist to make sure that people only use the whitelist when it's necessary. If they fail in principle the component should pass the normal check

[frank sigil]

compile errors in tests are 99% fixed

[oblique tusk]

what's the remaining 1%? 👀

[frank sigil]

PlayerXpTests because ExperienceOrb#value was replaced by direct entity data access and the setter is private

and GameTestTest

ok 1 class to go

just GameTestTest left

ok fixed by commenting it out

everything compiles now

we are just left with 5 reject files before we can go to the latest snapshot

[drifting finch]

@frank sigil nice

Did you remove the dig stuff?

[frank sigil]

tech did yes

[gleaming pendant]

Then just go to the next!

Put the rejects in another folder then update to the latest MC 😉

I want to get started on the client stuff when I get home later

[lunar saffron]

I'm gonna take a look at the remaining ones, they shouldn't be too bad to deal with except for the fluid ones

[oblique tusk]

fluids 👀

[frank sigil]

so how do we bump the snapshot

[oblique tusk]

a force push

(as in, why is there a force push)

[frank sigil]

I had to fix a pushed commit

[lunar saffron]

Summary since I can't actually push them now (😡):

• The LocalPlayer reject is entirely fluids
• The remaining hunk from the Player reject can be cleanly moved to BlocksAttacks#hurtBlockingItem()
• The LivingEntity reject is one hunk shield blocking (LivingShieldBlockEvent dispatch, needs some more thought as blocking is no longer a boolean flag, the BlocksAttacks component can decide how much damage it blocks), one hunk fluids and one hunk shield disabling (controlled by the Weapon component now, so the IItemExtension#canDisableShield() extension method should probably be removed)
• The BambooSapplingBlock and BambooStalkBlock rejects can be dropped as the destroy speed bonus is handled in the sword's Tool component now

[gleaming pendant]

Create a new branch then do it manually

[gleaming pendant]

Just cherry pick the diff

[oblique tusk]

smh for force pushes

[lunar saffron]

I hadn't even committed this yet, I'm just a bit annoyed that Schurli didn't wait a couple more minutes. I'm gonna wait for the update before pushing this

[frank sigil]

I had those commits ready before I read your message

I just had to fix an earlier one

[gleaming pendant]

Schurli are you doing the update?

Otherwise I'll do it in ~2 hours

[frank sigil]

no I'm currently not

[lost imp]

I'm doubtful you needed to rewrite the history to apply said fix

[gleaming pendant]

shit I forgot to gen patches before updating

ah but schurli did

great

alright let's see

41 reject files

seems to be mostly client stuff so I'm going to start there

nice, they got rid of the idiotic AttachmentState

vanilla is now using GL_DEPTH_COMPONENT32

(previously they were using GL_DEPTH_COMPONENT)

[drifting finch]

Nice

[lunar saffron]

Tech, I'm gonna start with some of the trivial rejects at the bottom if you don't mind

[gleaming pendant]

sounds good, I'm doing the top ones (all related to the stencil patches afaict)

[lunar saffron]

Sounds about right, yeah

[gleaming pendant]

should we keep the reduced stencil format as a config option?

or only support 32_8?

[fleet dome]

do we already have the config?

if yes: keep it.

[gleaming pendant]

yes we do

however previously it wasn't clear whether mojang might rely on 32 bits

[floral mesa]

We should probably only support 32_8 if vanilla explicitly uses 32 now

[lunar saffron]

That would be my thought as well

[frail patio]

Ping me for models

[gleaming pendant]

that was also my thought

looks like GlStateManager._clearStencil is gone

and... it's now back

[fleet dome]

Wat 😄

Ah!

Well I'll be honest, no one who'd actually need it will ever find that config option

[gleaming pendant]

🤷 it's still there for now

not good 😄

it looks like mojang removed all of the stencil-specific state tracking

we'll see what needs patching in when we get to fixing the tests ig

[lunar saffron]

Fixed almost all rejects in the world package, only remaining one is fluid nonsense

[gleaming pendant]

server looks easy enough

maybe client.renderer.texture for you? 😛

[lunar saffron]

Yeah, I'm handling the server package right now. I can do the texture stuff after that

[sterile abyss]

we need a new fml major to change the entrypoint of the gametest run config

[gleaming pendant]

again?

[fleet dome]

Can we finally just add proxy-entrypoint classes in NF 😄

Or just merge FML back in 😛

[sterile abyss]

well change is a strong word actually

add is the correct word

right now we hijack the normal server one iirc

[gleaming pendant]

remember: we considered it

[sterile abyss]

so I guess the question is whether we should bump the major to add a new launch target or not

[gleaming pendant]

the answer is probably that nobody cares 😛

and no we don't need to, it's kind of an internal API

[sterile abyss]

internal api is the best oxymoron

[gleaming pendant]

I'll give you another example

NeoDevFacade

[sterile abyss]

yes yes I know but the term is still funny :P

[fleet dome]

remember that we already have 7.x in a branch 😛

[gleaming pendant]

well we gotta start models at some point

first step I think is to add back hasAmbientOcclusion to BakedQuad

[lunar saffron]

The quad, element and element face rejects should be trivial

[gleaming pendant]

where does record field documentation even go?

[lunar saffron]

Above the class header IIRC

I've pushed the texture stuff. I'm gonna go play games for a bit and come back later

[gleaming pendant]

thanks 😄

[gleaming pendant]

so @frail patio where do we discuss models?

[frail patio]

If we use the Fabric thread it will be easier to find later but for actual discussion right now it doesn't really matter

[gleaming pendant]

what will probably happen is that every time we agree on something it gets implemented in neoforge 😛

it is quite interesting how vanilla only has two implementations of UnbakedModel

[frail patio]

True, but it also gets implemented in Fabric

Maybe not quite as quickly because I'm the only one that will write the code

[frail patio]

I assume there's a third one for the missing model

[gleaming pendant]

the missing model is a BlockModel

[frail patio]

Right

[sterile abyss]

can someone do me a favour and tell me the FQN of the new game test entrypoint

[gleaming pendant]

net.minecraft.gametest.Main

[sterile abyss]

thanks

[frail patio]

Let's just use the Fabric thread

[gleaming pendant]

sure

[river mural]

what is the difference?

[gleaming pendant]

GL_DEPTH_COMPONENT does not guarantee 32 bits

[frank sigil]

Can we please not have discussions about implementations that will be in neo happen on the fabric discord.
Just open a new post here in #1105595318197825557

[steep hamlet]

we users can't make a new post

[merry falcon]

that is a hindsight lol. Schurli, make the post

[glass wagon]

for the entity reject mojang moved the extinguish to Fluid.entityInside

[gleaming pendant]

Same argument goes both ways since Pepper also has to implement it in Fabric

[frank sigil]

well we are doing it now not somewhere down the line and if they are discussing it with us then it should be here not there

so please put a summary of everything that has already been discussed here #1342994966405845063 and continue there

[proven compass]

maybe there should be a common server or something where u can discuss cross loader compat

or a group chat or something

[frank sigil]

this is not about compat talk (yet) it is purely requirements and for networking we also did it here

both the main fabric maintainers are on neocord

[frail patio]

I am also doing it ASAP and in fact the Fabric implementation for the discussed and agreed upon ideas may be released to the public before Neo's is because Fabric API is released for snapshots

[drifting finch]

None of that, not even the compat with fabric matters, yet it is nice, but the especially for the core apis like this: Discuss it in here, or the rest of the maintainers will simply veto it.

That does not mean that you can not do research outside of the discord/NF or draw from experience in Fabric. Nor does it mean that you specifically pepper, or anybody else for that matter, can not first contribute anything to Fabric. But the flip side of that is, that we are under no obligation to accept the contribution. And this is really not something that needs major refactorings as far as I can see from kits (at least not within the 1.21.5 snapshot cycle, or maybe it does, the point is that is up to the community to decide)

I know that sounds harsh

But it is needed to protect our communities interest

Now if the change is easily revertable

Then that is a different story

Especially if those changes don't effect the wider community at all

So please use the #1342994966405845063 if the change is majorly significant

But if it is not, then please quickly explain and we can stop this weird arguing around and continue porting

[frail patio]

It is significant - that is why I wanted to participate in loader agnostic discussions

[drifting finch]

As with any API for NF, NF comes first, then we will try to make it loader agnostic

[frail patio]

The intent is not to make the code loader agnostic or to have the same code between loaders, but only to ensure the design choices of the APIs are the same or at least similar

[drifting finch]

I am sorry, but that does not matter either.

Because the NF community might have entirely different requirements then Fabrics, and that is okey.

Now if it turns out that by sheer coincidence (which happens quite often, see things like tags etc), then it is okey to cooperate, but the point of discussing it here in #1342994966405845063 (after potentially researching it outside, or drawing inspiritation from Fabric), is to determine that commonality between requirements.

So please talk in #1342994966405845063 and figure out

If the requirements line up

And if they do

Then you are in luck

[frail patio]

I am almost certain they do

[drifting finch]

Make certain that they do

And communicate that to our community

That is what we are asking for

That you communicate

And do it in the required channels

@frail patio To clarify this: I have personally nothing against this project, I think it is a good idea to make them parallel, and I also thing that they almost certainly do.

But as you can see from several others in here, and the fact that they want a discussion about this approach, several people are not as willing to accept our opinions at face value. So we need to convince them.

And we do that by following the protocol we agreed upon, with the entire NF community.

How stupid and useless with both think it might be

But that is how we build trust with the community

[haughty spade]

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

@marsh mural ^

[lunar saffron]

Oof, the loading overlay is going to be a bit of a pain in the ass. RenderSystem.setShaderTexture() expects either an RL or a GpuTexture but the only thing we get from the FML DisplayWindow is a GL texture ID. As it stands, we cannot create a GpuTexture from an existing GL texture and I'm not sure adding a constructor to do that is a good idea but we also can't tell the FML DisplayWindow to write to a different GL texture.

[fleet dome]

I can take a look at that ig

[lunar saffron]

Sounds good. I'll push the partial fix that handles the rest of the compile errors in the overlay

[fleet dome]

Which are we on 2508a?

[lunar saffron]

Yup

[oblique tusk]

i have switched the default branch to 25w08a

[lunar saffron]

I was about to ask about that 😄

[fleet dome]

Hm Xfact, where are you getting the shader texture from? the only reject in loadingoverlay i see is about the fadein

[lunar saffron]

NeoForgeLoadingOverlay line 118

[fleet dome]

god that code is fucking convoluted

[lunar saffron]

Yup 😅

[fleet dome]

Seriously, this is in FML

Class<?> clz = Class.forName(fm, "net.neoforged.neoforge.client.loading.NeoForgeLoadingOverlay");
Map<String, Method> methods = (Map)Arrays.stream(clz.getMethods()).filter((m) -> Modifier.isStatic(m.getModifiers())).collect(Collectors.toMap(Method::getName, Function.identity()));
this.loadingOverlay = (Method)methods.get("newInstance");

When can we finally move that stuff back into the repo 😄

So first gut-feeling is that we should indeed add a way to adopt an arbitrary GL texture into a GpuTexture

@lunar saffron you're not going to like it, but this should work™️
https://github.com/neoforged/Kits/commit/7d19d1373b141ab2dc2538485fa329ad533ec191

[lunar saffron]

To be fair, that's cleaner (and most likely safer) than what I would have come up with

[fleet dome]

What needs to be investigated once it compiles again: what does width/height actually do and why is it final

and how does it react to resizing the framebuffer

[lunar saffron]

I don't think the framebuffer used by the early display window is ever resized, it's just scaled to fit when it's drawn to the screen

[fleet dome]

I am not so sure about that, it definitely has resize callbacks (which may be borked, however)

but what does MC do internally? recreate the GpuTextures instead of just uploading a new image to the existing GL texture id?

We can likely adapt to that if we want to, although it might be slow. As long as we do this only once per frame it doesn't really matter though, I think.

(since it only occurs during load)

[lunar saffron]

There are resize callbacks for window and framebuffer resizes but neither of them actually resize the EarlyFramebuffer. Instead the values from the framebuffer resize callback are used to compute the scale in EarlyFramebuffer#draw()

[frail patio]

Alright, sure

[lunar saffron]

@fleet dome currently checking some other stuff, so I haven't dug any deeper into it, but format detection for externally controlled textures currently blows up:

java.lang.IllegalArgumentException: Couldn't find a matching vanilla TextureFormat for OpenGL internal format id 6408
    at TRANSFORMER/minecraft@25w08a/com.mojang.blaze3d.textures.GpuTexture.ofExternal(GpuTexture.java:332)
    at TRANSFORMER/[email protected]_2/net.neoforged.neoforge.client.loading.NeoForgeLoadingOverlay.<init>(NeoForgeLoadingOverlay.java:62)

[fleet dome]

OK, I'll have a look

did you guys gen patches?

[lunar saffron]

Nope

[fleet dome]

Does it compile? 😄

[lunar saffron]

Only locally on my machine 😅

[fleet dome]

It's complaining about not finding import net.minecraft.client.renderer.ShaderProgram;

Los! Schnell! Drücke!

[lunar saffron]

Yeah, I have a fix for the shader stuff which I'll push in a moment, just want to make sure it actually works

Pushed. The only remaining compile errors now should be GlStateManager and the composite model, both of which you can just comment out

[fleet dome]

Fascinatingly

GL_TEXTURE_INTERNAL_FORMAT returns the EXTERNAL format id

like... what the fuck

I mean it's a bit of a hackjob also comparing the external format, but it'll do for now

Gedrückt.

[gleaming pendant]

Pushed?

[lunar saffron]

Yup 🤣

[weak lichen]

Successfully scheduled reminder on <t:1740477120:F> (<t:1740477120:R>)!

[sterile abyss]

I forgot to do it on saturday, whoops

[glass wagon]

new neoform version for 25w08a with vineflower 1.11.0

[oblique tusk]

that was quick

[proven compass]

whats the difference

[oblique tusk]

looks like a good amount of NeoForm patches were made redundant

good

[glass wagon]

57 reject files for neoforge

[frank sigil]

we'll update tomorrow with the next snapshot

[weak lichen]

@sterile abyss

Camelot

Reminder

fml

matyrobbrt · Feb 25, 2025, 9:52 AM

[sterile abyss]

https://github.com/neoforged/FancyModLoader/pull/250

[frank sigil]

[weak lichen]

@sterile abyss

Camelot

Reminder

fml

matyrobbrt · Feb 25, 2025, 4:41 PM

[lunar saffron]

I've finally taken the opportunity to throw a hammer at the test framework UI rendering. The immediate rendering has been pissing me off for several versions and it finally broke completely now. The cases I found have now been replaced with proper usage of GuiGraphics

Current state of the port:

• Loading overlay rendering is broken once it switches from FML-controlled to the NeoForgeLoadingOverlay (see screenshot). This may caused by how I fixed the compile errors, will investigate later
• The vanilla gametest side of the Neo gametest framework is fucked at the moment. GameTestInstances are a datapack registry and the ones created by the framework currently return a null codec, blowing up the registry sync
• The stencil extensions, if enabled, cause a GL_INVALID_OPERATION in the GpuTexture constructor when the special Fabulous mode render targets are set up
• There are two remaining rejects, one related to fluids and one related to fluids and shield blocking

[weak wasp]

I was really hoping mojang would add a bit more than just 'always pass' as a datagen option...

I'll probably write out the game test in the primer by pre1 if they don't though

[sterile abyss]

ah they're synced?

I guess we'll have to make them a proper object

blergh

[lunar saffron]

Yes, test environments and test instances are synced

[lunar saffron]

Ok, I'm fairly certain I know what the GL_INVALID_OPERATION is about: if I disable the eager error checking in the GpuTexture constructor and instead let the debug callback spam the logs, then I get the same "depth format does not match" error we got during the 21.3 or 21.4 port. The special-case of TextureFormat.DEPTH32 in the GpuTexture constructor looks suspect to me but adjusting it to also check for the custom depth-stencil formats we added doesn't fix it.

[lunar saffron]

Ok, lots of debugging later (i.e. spraying GL11.glGetError() != 0 -> throw statements all over the place) I have found that this is a red herring
The actual error happens when the translucent target copies depth from the main target in LevelRenderer#addMainPass() right before translucent chunk geometry renders but since nothing calls glGetError() between that and the GpuTexture creation for another TextureTarget, it gets caught there and blows up.

[frank sigil]

I will probably need to create 1 or 2 example galetests for the neoforge docs anyway so whoever of us comes to it first should bear both use cases in mind

[frank sigil]

I can look at this tomorrow if you don't have time for it till then

[fleet dome]

I still have compile errors in GlStateManager, xfact

[lunar saffron]

Yeah, we still need to decide how to solve that, i.e. which of the removed methods we need to reintroduce (mainly for the stencil state). I currently just have it commented out

[fleet dome]

I'll just restore that stuff real quick

essentially just copy pasta from the previous version

[lunar saffron]

Ok, finally found the cause: we got rekt by the resource re-use through CrossFrameResourcePool. It checks whether a given existing resource can be used according to the ResourceDescriptor passed to acquire() by calling ResourceDescriptor#canUsePhysicalResource(). The implementation of that method in RenderTargetDescriptor did however not compare the stencil state and therefore re-used a TextureTarget with stencil disabled

[fleet dome]

Ok sounds very plausible to have missed that yeah

what does GL_POLYGON_OFFSET_LINE even do hm

[lunar saffron]

Not a missed patch, that method didn't exist in 1.21.4 and the descriptors were instead compared by calling equals()

[fleet dome]

That's what I mean though

[lunar saffron]

Ah, I thought you meant that literally, i.e. you accidentally dropped a patch

[fleet dome]

Ah no, I meant missing a new use, which are sometimes the hardest to get right

poly offset line wasn't even exposed in 1.21.4 as far as I can tell

The only uses:

[lunar saffron]

No point in adding it back then

[fleet dome]

@lunar saffron https://github.com/neoforged/Kits/commit/f9e229d1a4a574521e6ed140bef70978e3c07852

Compiles for me now

[lunar saffron]

Looks good to me

One additional thought from this whole ordeal: the last thing I did which finally pointed me at the line that was causing the error was to enable synchronous GL debug output by passing true as the second param to RenderSystem.initRenderer() in the Minecraft constructor and putting a breakpoint in GlDebug.printDebugLog(). I think it would be a good idea to add a config option to enable this because it significantly simplifies debugging these kinds of errors.

[floral mesa]

And here I was manually configuring OpenGL and hacking in a custom error handler that dumps stacktraces to enable that

[lunar saffron]

To be fair, that would be easier to notice if GL constants were unpicked. The marked line is what enables the synchronous output

[frigid sand]

magic constants go brrr

[lunar saffron]

Regarding the stencil stuff: should this line also check for our custom depth-stencil formats: https://github.com/neoforged/Kits/blob/25w08a/projects/neoforge/src/main/java/com/mojang/blaze3d/textures/GpuTexture.java#L55?

[fleet dome]

Same

I knew about the parameter already and in the discussion about the setup stencil event, I proposed to make it SetupRenderSystem and allow synchronous debug to be enabled 😛

But TBH, config option sounds good to me

[floral mesa]

Config option is probably better, it seems unlikely anyone wants to be able to turn that on from a mod outside dev (and if they really have some use case for it, they can just ModifyExpressionValue in RenderSystem themselves)

[tardy leaf]

Fun fact: in source code one of SharedConstants fields is wired directly into that, because it's pretty much the intended use

[lunar saffron]

Makes sense that a field for it would hide in that class 😄

[fleet dome]

didnt you want to do that at some point 😅

[lunar saffron]

Now the question is just: which config do we add it to? The client config loads too late and adding a dedicated startup config for just one thing seems a bit overkill. I guess we could add it to the FML config

[floral mesa]

We have other GL configs in there, right

[lunar saffron]

Yes, but all of those apply to the early window directly, this one wouldn't. Probably not an argument against adding it there though

[lunar saffron]

Made a PR to add an FML config entry for this: https://github.com/neoforged/FancyModLoader/pull/251

[sterile abyss]

god I hate that so much

adding a config option in a repo to actually use it in another

so much pain

[fleet dome]

Wait why ... oh.... blerrrrrrrrgh

[lunar saffron]

[haughty spade]

/me shudders

[gleaming pendant]

Re stencil do we need to patch anything back in for now?

Some of the opengl stencil methods did already not have a RenderSystem equivalent I think (or was it a missing state shard?)

Well fine to have it ig

[fleet dome]

I think I did pull in most of it again

It was removed by Mojang (for not being used, most likely)

But yeah to make it realistically usable it should be a state shard

[gleaming pendant]

Maybe, I'm not even sure. Stencil operations are highly timing sensitive

I did have to patch clearStencil back in after all so yeah you probably did roughly the same thing

[fleet dome]

Wat 😄

So are all draw operations

The bigger isue is likely that it's more a pass-thing than an individual render state shard

[gleaming pendant]

That's what I mean

They are highly dependent on the state of the stencil buffer

It wouldn't make sense to batch them

[fleet dome]

well yes, the issue is not really timing it's that what Mod A uses as a stencil value can be completely meaningless to Mod B

[gleaming pendant]

Yup that too

[lunar saffron]

Stencilling is basically identical to scissoring in this regard and vanilla keeps the latter separate of render types as well.

[lunar saffron]

Speaking of stencilling though: do we need to patch this ^? I have no clue what that GL call does

[fleet dome]

Not quite, you can set stencil values from shaders

[lunar saffron]

Interesting, I didn't know that

[vagrant obsidian]

Wonder what will possibly break today

Anyone have that list of items that still have dedicated classes?

[limpid crypt]

https://www.minecraft.net/en-us/article/minecraft-snapshot-25w09a

fyi

[winged salmon]

nice

fallen trees

[limpid crypt]

yeap

[cinder saffron]

Ah, interesting. Lots of backend updates, I'm guessing?

[marsh mural]

Changelog: https://www.minecraft.net/en-us/article/minecraft-snapshot-25w09a
SnowMan: https://github.com/neoforged/Snowman/commit/9b69abbdede3c201e70bf8553e8b555195aa5444
Primer: https://github.com/neoforged/.github/blob/15ead8eb9c9c43bee91073097e3f2e7ccb103906/primers/1.21.5/index.md
-# Changes: https://github.com/neoforged/.github/pull/14/commits/15ead8eb9c9c43bee91073097e3f2e7ccb103906

Slicedlimes Videos:

• Main: https://www.youtube.com/watch?v=6PK34Qpl5Lk
• Data/Resource Pack Changes: https://www.youtube.com/watch?v=E_NCyRsdX5g

<@&1067092163520909374>

[frigid sand]

debug labels

Bit more space between numbers could be useful

[arctic bane]

Datapack version

[marsh mural]

https://tenor.com/view/noice-gif-21882904

[frank sigil]

NICE

[marsh mural]

snowman link added to pin

[lunar saffron]

I smell Vulkan

[frank sigil]

I feel like that reference is on purpose

[unkempt lynx]

models are fucked again

block models this time

[frank sigil]

@river canopy

[river canopy]

none of that looks like vulkan to me?

[thin cairn]

Yeah it's 100% a reference to the good old "all local variables are ⛄"

[tardy leaf]

https://unicodesnowmanforyou.com/

[river canopy]

the fact that you're using DSA though is great :D

[lunar saffron]

The abstraction looks very much like prep for Vulkan to me

[river canopy]

sure ig

[thin cairn]

TIL Character#codePointOf(String) exists

[river canopy]

speaking of Vulkan, I have a bunch of nice things including bindings that are more platform-independent than LWJGL

specifically, I don't need any natives at all

[frigid sand]

[thin cairn]

Let me guess, Panama?

[river canopy]

Mhm

[thin cairn]

And jextract?

[river canopy]

No

jextract's bindings aren't well suited to Vulkan

Instead I'm generating the Vulkan bindings directly from the vulkan spec

[thin cairn]

Ok well at least you're generating them

[river canopy]

A lot of it is also hand-written, because the vulkan spec is really badly written

[thin cairn]

Yeah especially since the package is named opengl. Smells like they might want to add other backends

[merry falcon]

Hold on. The mushroom is in the open bright light

[thin cairn]

Maybe even Metal so Mac users aren't dying in ancient versions of APIs

[merry falcon]

DID THEY CHANGE MUSHROOM SURVIVAL CODE

[river canopy]

Incredibly unlikely

They're almost certainly going to use MoltenVK since that's a direct part of Vulkan

[thin cairn]

Fair

It's just that some of this feels like they might want to actually have multiple backends available

[frank sigil]

soo is neo compiling atm? if so, gen patches, create new branch, update

[river canopy]

From experience, you rarely want to maintain multiple backends as it makes the code a lot harder to reason about

The abstractions mainly exist for supporting migrating to another backend API, or for when one API is unsuitable

e.g. on platforms where Vulkan is unsupported (which is depressingly many still)

[thin cairn]

Many game engines do just that though, as it allows them to use the most optimal backend for the platform

[river canopy]

That's not at all why they have it 😜

[thin cairn]

So for example, DirectX on modern Windows, Metal on macOS, Vulkan where supported, and OpenGL elsewhere

[lunar saffron]

Neo compiles, yes. Go for it

[thin cairn]

Then why do they have it?

[river canopy]

The entire reason things like Unreal's RHI exist is to enable game developers to migrate from, say, OpenGL to Vulkan or from DirectX 9 to DirectX 12

Or from proprietary per-platform rendering APIs (like those on Nintendo and Sony devices) to industry-standard ones

It's nothing about performance; in fact if you speak to the people who use them they'd really prefer you use just one and stick with it, because it makes your code easier to reason about

the reason it exists is to make it possible to migrate in the first place when APIs come into and out of use

like 3dfx' Glide API for example, back when 3dfx were a thing

[thin cairn]

My experience is with Unity, which does what I said

Iirc

By default anyway

[river canopy]

Unity used to be like that, yes

but with the newer URP/HDRP they're "encouraging" people to stick with either Vulkan or DirectX

[thin cairn]

Ah

It's interesting what Mojang is choosing to mark @DontObfuscate. Like why is DestFactor @DontObfuscate?

[river canopy]

Could have been a workaround or a mistake

[weak wasp]

Ok, I know why mojang has done this, but shame on you for removing the GL codes

[thin cairn]

How far have you read?

[river canopy]

the GL codes?

[thin cairn]

Because they didn't remove them

They're in static methods in another class now (GlConst)

[weak wasp]

Currently looking through, but basically they're not directly attached to their enum objects

Which is what I dislike

[thin cairn]

You just call GlConst.toGl(yourConstant)

[weak wasp]

I know

[thin cairn]

Apparently according to VineFlower, all those methods return kanji

[weak wasp]

oh fun

[river canopy]

ah, so you mean stuff like NativeImage.Format no longer having the GL format constant?

or BufferType/BufferUsage not having them?

[frigid sand]

decompiler having fun?

[river canopy]

Yeah, presumably that's the compiler and/or proguard applying some bytecode transformation to make it load a char which then gets upcasted to an int

[thin cairn]

One of them returns a unicode combining macron

[thin cairn]

There's no char constant

The smallest constant is an int

So the decompiler is guessing (incorrectly) that it's supposed to be a char

[thin cairn]

'̄'

[frank sigil]

are you using the newest VF?

[thin cairn]

No

Lemme poke the person who manages this repo

[weak wasp]

Yeah, I understand why, but it does make it a bit annoying for people unfamiliar with the abstractions

[river canopy]

there's bipush and sipush, as well as the whole cursedness that is wide

Oh one sec, I misread bipush

[thin cairn]

Those both return an int

[river canopy]

that's for bytes

[thin cairn]

bipush means byte int push

sipush means short int push

Bytecode has no concept of byte/char/short outside of field and method descriptors

[river canopy]

i2s would like a word with you

[sterile abyss]

(it also doesn't need to compile to gen patches)

[thin cairn]

Returns an int

[river canopy]

(which is also cursed)

[thin cairn]

It just sets the top 16 bits to 0

[gleaming pendant]

@frank sigil let's do the update to newer vineflower separately from the update to this snapshot

[cinder saffron]

I noticed that too! There’s already a tag for “mushroom survives on”, so I assume they just added logs to it.

[river mural]

Should we switch to vine flower on the existing snapshot then before porting to the new one? Especially as a Neo form branch already exists

[glass wagon]

https://github.com/neoforged/NeoForm/actions/runs/13549029759/job/37868044831#step:8:236
Apparently vineflower can't successfully decompile the new snapshot

[tardy leaf]

arms race

[gleaming pendant]

I guess we see if coeh can fix it for the new snapshot

[tardy leaf]

and we are not even trying

[gleaming pendant]

But yeah we shouldn't mix the 50 or so vineflower rejects with the snapshot logic changes

[glass wagon]

And we already updated vineflower earlier this week

[gleaming pendant]

On Kits?

[glass wagon]

Neoform

[gleaming pendant]

Well it will cause rejects in Kits which should be handled separately from the rejects coming from the snapshot

[river mural]

^ which is why I suggested starting with updating to the new vine flower as it is already updated for the previous snapshot on neoform

[lunar saffron]

GitHub is not happy about the amount of changes to this file

[vagrant obsidian]

Soooo many changes to depth and framebuffer code..

Gander and CC are gonna get rekt

[thin cairn]

CC?

[vagrant obsidian]

Compact Crafting

[thin cairn]

The mods I know by that name are Cubic Chunks and ComputerCraft

[thin cairn]

Ah oof. Doesn't seem to exist past 1.19 though

[vagrant obsidian]

It does

It's been in rewrite hell due to all the rendering and capability changes

[thin cairn]

Ah

[vagrant obsidian]

also

Mojang please. These are enums. WHY ARE THESE NOT IN THE ENUM

[thin cairn]

Because abstraction

At some point there will probably be other backends with different constants

[vagrant obsidian]

DepthTestFunction.toGL() / toVulkan() / .toDX12() was too hard?

[thin cairn]

The backends should be pluggable and not hardcoded into each constant class

That goes against the whole point of pluggable backends

[vagrant obsidian]

Functional interfaces exist, then

Besides, YAGNI

They could have started with the methods on the enums themselves, THEN moved it once more mature

[sharp haven]

We did. 🙂

[thin cairn]

Yeah it was like that in 25w08a

[vagrant obsidian]

...aight

I don't see any kind of FI/swapping logic, but I guess that'll come later then

Still, the rendering logic looks to be cleaning up in huge swaths, so congrats

🎉 A bunch of NBT classes got recordized

[trail fable]

woot

[vagrant obsidian]

Also SnbtGrammar exists now

[frail hemlock]

Hello, there is now a vineflower snapshot [1] that should fix the decompiler crash when processing SnbtGrammar
[1] https://s01.oss.sonatype.org/content/repositories/snapshots/org/vineflower/vineflower/1.11.1-SNAPSHOT/vineflower-1.11.1-20250226.180959-2.jar

we are aiming to get the full release out in a couple days

[gleaming pendant]

Cool thanks a lot!

[frail hemlock]

I also noticed this issue, I am aiming to fix it in the aformentioned next release

[vagrant obsidian]

There's an absurd amount of changes that boil down to CompoundTag getting getXXXOr methods (flattening the if-contains : get : else pattern all over the place)

Abilities.CODEC soon, or perhaps these will move to entity components

If I had to guess, looks like Mojang is pushing towards immutable NBT

And CompoundTag is standing in the way (it hasn't been recordized yet)

Probably going with their work on making NBT only exist in persistence and components/StreamCodec being the in-memory form

[glass wagon]

Switch statement

[steep hamlet]

New command line argument --renderDebugLabels is available for the client
Adds debug labels to relevant OpenGL objects, making debugging rendering easier for developers
NEAT!

[steep hamlet]

Dinnerbone has, a few days ago, told me, that they are not adding vulkan

[raw cave]

"Vulkan stuff" and "taking steps to make an eventual migration to vulkan easier" are not the same thing though

also yeah, gl is not getting updates

[vagrant obsidian]

They're abstracting

To prepare for stuff like macOS completely dropping OpenGL support

[steep hamlet]

yes
but I have seen a few people here saying they are having vuilkan stuff in MC

[vagrant obsidian]

Wrong

[raw cave]

[vagrant obsidian]

We're saying they're abstracting, PREPARING for stuff LIKE Vulkan

[raw cave]

do those people look at code

[weak lichen]

xfacthd

[Reference to](#1313560104359956542 message) #1313560104359956542 [➤ ](#1313560104359956542 message)I smell Vulkan

finnt730 referenced · Feb 26, 2025, 4:05 PM

[frail hemlock]

I saw this, the switch-on-enum is being decompiled as a record pattern match switch

[vagrant obsidian]

BTW

[frail hemlock]

i think this is why the class was breaking in the first place

[vagrant obsidian]

CompoundTag#contains is gone

Prep for that nowish, I guess

[steep hamlet]

and I do believe I have seen Comm talk about it before somewhere too (or tele)

[vagrant obsidian]

(can we backport the new store/load stuff to 21.4 now)

[sterile abyss]

?

[merry falcon]

Excuse me, I don’t talk rendering

[sterile abyss]

it's not gone

[weak wasp]

Ok, I'm loving these changes

[steep hamlet]

then it might have been comm
I have seen it before in one of the 40 channels of this server

[weak wasp]

Mainly because it doesn't seem like I'll have to rewrite the primer that much

[steep hamlet]

Integer numbers can now be prefixed with 0x to represent hexadecimal numbers and 0b to represent binary numbers
Example: 0xbad (equal to 2989), 0xCAFE (equal to 51966), 0b101 (equal to 5)
neat!~

[lunar saffron]

If you read further, you would have noticed that I clarified this comment to refer to the fact that the abstractions look like prep for Vulkan (or something similar if you want to be pedantic).

[vagrant obsidian]

These have all moved to return Optional<T> instead

[raven blade]

In json strings?

[sterile abyss]

there's still a contains(String)Z

[steep hamlet]

I normally do not read further, and then scroll all the way back up
to much effort to double scroll

[tardy leaf]

I have 99 problems, but Tag.NUMERIC_TAG is not one of them

[sterile abyss]

and a get(String)Tag?

[lunar saffron]

Then stop generalizing people's statements if you're unwilling to read the surrounding context

[sterile abyss]

I assume you had 100 problems and fixed one of them to end up with such a nice number

[thin cairn]

Can't wait for value record ByteTag when Valhalla eventually becomes real

[sterile abyss]

never!

[vagrant obsidian]

CompoundTag#getTagType also gone

These are nice adds tho

[sterile abyss]

the removal of type oriented methods is consistent with the helpers

dealing with the different types was indeed annoying

[raven blade]

Aww, nbt... still nice

[sterile abyss]

isn't mojang deserializing with lenient gson?

[thin cairn]

Not that I know of

[limber ginkgo]

hm optionals

[tardy leaf]

some legacy parts used lenient
last place got removed two or three releases ago

good riddance

[sterile abyss]

I think recipes were back before codec recipes

[limber ginkgo]

honestly i gotta say i kinda prefer the if contains then syntax, optional has extra overhead that isn't really necessary

[thin cairn]

I like that lenient Gson has comments though...

[sterile abyss]

tbh lenient gson is nice for comments

[haughty spade]

change to a json5 library next, boq, so people can write comments in their resource packs, and skip the quotes on simple keys ;p

[sterile abyss]

so I think it's a downgrade :P

hmm private static final Gson GSON = new GsonBuilder().setPrettyPrinting().disableHtmlEscaping().create(); that's 1.20.2 recipes

[vagrant obsidian]

You're talking about serialization code though, it's not in hot code

And I wouldn't worry about that until someone shows real numbers that it's a problem

[sterile abyss]

if it's not recipes, what was lenient then

[unkempt lynx]

Gson is a JSON5 library

they just disable it

[thin cairn]

Gson lenient is not json5

[haughty spade]

isn't lenient just a "on error skip character"?

[thin cairn]

It's a whole lot less defined and a lot more jank. It also doesn't support stuff that json5 supports, like hex numbers

[unkempt lynx]

it's close enough

[unkempt lynx]

no one will notice

[trail fable]

not really

[thin cairn]

Yes a lot of people will notice

[trail fable]

if you tell people it's JSON5, people will expect it to be JSON5

and not, JSON with a few extra things but not everything JSON5 has

[thin cairn]

0x10 in json5 decodes to 16. In Gson lenient it decodes to "0x10"

[unkempt lynx]

fine

use moshi or something

[thin cairn]

let's not pull in the Kotlin stdlib

[trail fable]

might as well just... use a proper JSON5 library?

[glass wagon]

Here's what I think the switch expression should look like

[trail fable]

oh I have an awesome idea, Mojang should write their own JSON5 lib! /hj

[thin cairn]

Those don't exist for Java afaik outside of Quilt Parsers, which is unmaintained and not on Maven Central

[sterile abyss]

written using their packrat parser

[limber ginkgo]

sure, but it's still another object allocation and the lambdas just bloat bytecode size

[vagrant obsidian]

I doubt anyone cares about bytecode size of Optionals when you have games like RD2 chewing through 200+ GB

[raw cave]

no no we should clearly change datapack files to css

[raven blade]

hey, the json5 argument is back around! fun to see a few people argue over it about once a year

[sterile abyss]

should make it a bimonthly event

[thin cairn]

And as far as allocations, I'm pretty sure those ✨go away✨ because of JIT magic

[thin cairn]

Like EMI's config :P

[lunar saffron]

Also worth noting that the optionals are only allocated in the methods without a default value

[raven blade]

configs should be structured LLM prompts and then it gets changed according to another NLP agent

[raw cave]

yes that's the one i'm referencing (it's godawful)

[weak wasp]

Well, it seems like BakedModel has been partially brought back with BlockModelPart

[limber ginkgo]

that is... a bad mindset, that's like me saying "why should i bother to optimize my game, the 5090 exists"

sure, it exists, does everyone have one? can everyone afford one? no, just because someone else does something it doesn't mean you should do the same

[raven blade]

hey, seems like that's where the AAA industry is now anyway, you're just being up to date with the current workflow

[thin cairn]

There's something to be said for optimizing. There's also something to be said for not over-optimizing such that it harder to develop

If it makes it easier to develop with minimal cost, go for it

[limber ginkgo]

i don't necessarily see what's harder having to nullcheck

[vagrant obsidian]

While I hate the comparison, this is exactly where Lex's "numbers or STFU" argument comes in. Sorry, but give real world impact

[thin cairn]

Bytecode-wise, these make up I'm sure no more than a few kilobytes in the entire codebase

[vagrant obsidian]

Until then, there's WAY bigger things to worry about

[limber ginkgo]

it's basically the same, your just creating extra objects for no real reason

[thin cairn]

In a game that is tens of thousands of kilobytes, and that's even ignoring audio assets

(the game JAR is like 30 MB)

And the entire game is still smaller than games that came out years before it even began development

[vagrant obsidian]

Anyway, RIP all rendering and custom data logic mods

21.5/22.x will be wild

[thin cairn]

Yeah RIP my code that renders blocks in a GUI

[vagrant obsidian]

You'll have Gander for that, once we pull it out of ITS grave

[thin cairn]

Nah I'll just do it myself

I only render one block at a time

It's not worth adding dependencies over. My only deps are loader APIs, and I port to RCs

[pearl needle]

hopefully there will be some libs to make the transition easier

[vagrant obsidian]

I mean

I already mixin'd a bunch of the codec utility methods into my 21.1 code

You can prep now, easy

[marsh mural]

Added slicelimes changelog video

[weak wasp]

Primer will likely be done by the weekend, definitely not today with the nbt changes

[frigid sand]

you don't communicate purely via images?

[gleaming pendant]

I'm gonna clean up the 24w08a port before moving on already

do the tests compile and run?

[lunar saffron]

Compile yes, run theoretically. The test framework is fucked (I had to comment out both dispatches of RegisterGameTestsEvent to even get into a world)

Regarding the ModelBakerExtension rename: make sure to change the interface injection file as well

[gleaming pendant]

ah you already fixed all checks?

[lunar saffron]

Yup

[gleaming pendant]

so gradlew build works?

Exception in thread "main" java.lang.module.ResolutionException: Module fml_loader reads another module named fml_loader

well we'll see another time

[lunar saffron]

Seems to only blow up on missing license headers

[lunar saffron]

I forgot to mention that, seems to only happen for client data though

[river canopy]

My instancing geometry pipeline will probably not have a good time with the model changes... hopefully i'll be fine

[lunar saffron]

Or more specifically, all formatting checks

[gleaming pendant]

did you fix spotless?

[gleaming pendant]

ok we can look into it another time

(another snapshot)

[lunar saffron]

I ran it, I can push it if you want

[gleaming pendant]

👍

yes please

>gradlew check
Reusing configuration cache.

> Task :neoforge:compileJava FAILED                                                                                                                                     
Unrecognized option: -Xlint:-deprecation
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

wtf now 😄

[lunar saffron]

[gleaming pendant]

@frank sigil

reverting these changes

[frank sigil]

just comment them out they are nice for initial compilation after fixing the patches to see the actual total of compile errors without deprecation stuff mixed in

[lunar saffron]

You're adding them to the wrong thing, they should be in options.compilerArgs, not the fork options

[frank sigil]

nope they need to be on the jvm according to the stackoverflow and my testing

[gleaming pendant]

why would java recognize these options

[thin cairn]

The error is that they're in the wrong place

[frank sigil]

why wouldnt it? theye are builtin to javac

[lunar saffron]

They are not JVM options though, they are compiler options

[frank sigil]

but they are somehow not picked up when adding them as compiler options

[lunar saffron]

Worked fine for me for the maxerrs option ¯_(ツ)_/¯

[thin cairn]

java {jvmArgs} -m jdk.compiler/com.sun.tools.javac.Main {compilerArgs}
The java command-line doesn't know what linting is. That's for the Main to pick up

[gleaming pendant]

we'll need to update the AT at some point

but maybe not right now

[frank sigil]

then tell me why it works the way I did it?

[gleaming pendant]

it doesn't

[thin cairn]

Yeah it doesn't. It only does for you for some reason

[gleaming pendant]

it might have "worked" if your JAVA_HOME is java 21 but that's only because it didn't fork

so the fork options are not even used

[frank sigil]

but why did it no longer report the deprecations and show more than 100 errors once I added it?

[thin cairn]

Isn't 100 the default?

[frank sigil]

100 is the default max yes

[gleaming pendant]

updating to new neoform now

I fixed injected interfaces, hopefully; too lazy to look into the other patch format violations

alright we have a quick reject fiesta

come and take part! 😄

I'm starting from the top (n.m.client)

[lunar saffron]

I'm starting from the bottom

[gleaming pendant]

second one is incorrect, right?

at least they should be consistent hmm

[lunar saffron]

Yeah, the second one is incorrect, it shouldn't wrap it in translationArg()

[gleaming pendant]

thanks

only n.m.world now

actually only entity heh

[lunar saffron]

Yeah, I'm digging through that right now

[gleaming pendant]

meet in the middle then 😄

I start with ai (top)

[sterile abyss]

https://github.com/neoforged/Kits/commit/ca636d0623dde695e6ecdd9661284c8484daf277#diff-8da9e64d53d0492cff41d5dcdf01e6fea4ee3b2174d342c2ff31900fbbe80e75R19-R24 this is such a weird patch, why are we deprecating the constructor

[lunar saffron]

Will do

[sterile abyss]

I'll take a look at fixing the game test function

[gleaming pendant]

it's the old one

maybe we shouldn't idk

[sterile abyss]

yeah I know but it's a weird one regardless, the list is an optional param

[sterile abyss]

and bump fml to add the new launch target

[gleaming pendant]

I wonder why we reorder like this

ah because the stack gets consumed in usePlayerItem, makes sense

[frail patio]

How does it work in vanilla?

[gleaming pendant]

no idea 😄

ok I stopped after Creaking

you can do Player ig 😄

[lunar saffron]

Yup, I'm on that right now

[gleaming pendant]

@glass wagon no neoform release yet? 😄

in any case I'm going to sleep now, will port kits to new neoform tomorrow 😄

had some fun with our little meet in the middle, XFact 😂

[lunar saffron]

Hehehe

[gleaming pendant]

(even though you got stuck inside a n.m.world-shaped hole)

[drifting finch]

Nice work boys!

[lunar saffron]

Sounds good to me

[drifting finch]

Hands down!

[sterile abyss]

so many ats

@lunar saffron this one seems not to generate anything?

RenderPipelines is under net.minecraft not com.mojang and all fields already seem to be public?

[lunar saffron]

The fields are public because I made them public in anticipation of the AT

[sterile abyss]

ah

I'll fix the target then

the new VF version resulted in quite a few decomp diffs

over 200 patches changed

[lunar saffron]

Yeah, the primary difference seem to be significantly less redundant casts and similar things (i.e. Boolean.valueOf() and friends as well as int to double casts and comparable) and more pattern matching instanceofs

[sterile abyss]

patches genned, AT generation fixed and invalid ats removed

we can update tomorrow to the next snapshot now

[lunar saffron]

Did some further cleanup on the updated patches (yes, I'm pedantic, sorry, not sorry)

[glass wagon]

and now I have to debug why I am getting a NullPointerException whenever I open the single player world list screen

[glass wagon]

rendertarget.width and rendertarget.height are int and renderpass has 2 methods named setUniform with the first taking an int vararg and the second taking a float vararg. The bytecode calls the method that takes a float vararg but vineflower doesn't include the casts to floats making this instead call the int vararg method

[gleaming pendant]

Cannot find launch target neoforgeclientdatadev, unable to launch huh

did we deprecate it?

probably some other problem causing this heh

ah looks like I'm using a broken ClientData run from the past 💀

[drifting finch]

Oef

[gleaming pendant]

ok datagen does actually run, that's great 😄

[drifting finch]

Very nice!

[gleaming pendant]

@lunar saffron since you also ran into this: the ClientData runs are legacy that should be deleted from your IntelliJ

(we have a run of type clientData, but its name is Data)

[gleaming pendant]

30 reject files

most under client or blaze3d

the others are probably NBT related

Codec<BlockStateModel.Unbaked> CODEC 👀

[glass wagon]

The get methods in CompoundTag now return an Optional

[fleet dome]

oh gawd

[haughty spade]

but we don't have valhalla yet

[sterile abyss]

<@&1067092163520909374>

[steep hamlet]

Oh no, oh no, ohhhh

[jovial dome]

https://mdcfe.dev/mc-changes?ver=25w09b

[frigid sand]

[steep hamlet]

@sterile abyss link it

[fallow merlin]

1pm in Sweeden 😮

[frigid sand]

https://www.minecraft.net/en-us/article/minecraft-snapshot-25w09b

[fiery tusk]

a bit of crashing issues solved

[marsh mural]

i just woke up and theres a new snapshot

Changelog: https://www.minecraft.net/en-us/article/minecraft-snapshot-25w09b
SnowMan: https://github.com/neoforged/Snowman/commit/420e8a981f6b69f3fa5cad4fe8657d2a883ac248
Primer: https://github.com/neoforged/.github/blob/6e283d34e1b5fd7a9d0e870750cbd3f3eefe7f4d/primers/1.21.5/index.md
-# Changes: https://github.com/neoforged/.github/pull/14/commits/6e283d34e1b5fd7a9d0e870750cbd3f3eefe7f4d

Slicedlimes Videos:

• Main: https://www.youtube.com/watch?v=SD6C2ZPHLYM
• Data/Resource Pack Changes: N/A

[raw cave]

They added thermal's blizz to vanilla?

[marsh mural]

in the thumbnail, thats the breeze, no? been in a for a version or 2 iirc

was added with the trial chambers and such iirc

[soft pivot]

yep

[frigid sand]

yes, because wardens and breezes crashed the game

[marsh mural]

damn warden not only reks me but also my game now (or did)

[frigid sand]

and going to the end through the nether also crashed the game, that's why the portal is there

[winged salmon]

First b snapshot in a while

[limpid crypt]

24w05b iirc

ah no

24w21b

[haughty spade]

oh hey a b snapshot

[weak wasp]

Primers time
25w09a: https://github.com/neoforged/.github/pull/14/commits/15ead8eb9c9c43bee91073097e3f2e7ccb103906
25w09b: https://github.com/neoforged/.github/pull/14/commits/6e283d34e1b5fd7a9d0e870750cbd3f3eefe7f4d

@marsh mural

For the general overview, pipelines have been further abstracted into the GpuDevice system, tags and their associated packrat parsers have received a makeover, and the readdition of BlockModelPart for a baked representation of a block model.

[marsh crescent]

I wonder if the Mojang team ever reads through this chat as a team with a cup of coffee and just goes "hmm how are they taking the changes?"

[dark wasp]

it's like knocking over a chicken coop and watching all the chickens run around yelling

[haughty spade]

I don't know about the coffe, but the rest definitely so

well "as a team"

I presume they have some internal chat

discord or otherwise

and they comment on things as they read the channels

[warm night]

I bet it's MS teams

[marsh crescent]

If i ever got a job at Mojang i'd for sure be like "<neoforge team member> is gonna hate me for this *commit*" i'd probably add commit messages like "sorry giga"

[merry falcon]

Slack

Mojang uses Slack. Not Teams

Why? Good question

[warm night]

not much better lol

tho it is better than MSTeams

[marsh crescent]

i use teams for work, and honestly don't have much negative to say about it.

[limber ginkgo]

teams is actually the worst thing ever

[drifting finch]

THe modern teams is fine

I use it everyday and it gets its job done

Is it bad no

Is it great no

But the same can be said about discord

[tardy leaf]

damn
who leaked out development practices

[river mural]

The fox who tried to sneak into the chicken coop, duh

[thin cairn]

I use Slack at work. It's fine

It does what it needs to do

[stiff flicker]

it's so bad haha I use it for work too and it absolutly sucks

[frank sigil]

Remember when I visited the office in fall...

[proven compass]

why does a microsoft company not use teams

[dark wasp]

same reason openAI doesn't allow job applicants to use chatgpt to write resumes

[thin cairn]

They're owned by Microsoft, but run pretty independently

So same reason they use Jira, not Azure DevOps

[outer stratus]

If you want to link to my changelogs, you should really just put the release version in the url instead of the latest snapshot (which will most often be empty since it takes a bit to write them), and 1.21.5 combines the changes from all snapshots even when the version isn't out yet.

[weak wasp]

Okay, I just remember checking I think a year or two ago and that wasn't the case, though I could've just checked too early at the time

[stiff flicker]

are you guys still having issues with decompiling the snapshot?

[lunar saffron]

Not anymore, we're using the VF snapshot version Jasmine linked yesterday ^

[stiff flicker]

Oh good!

[marsh mural]

hmm, do others think i should add this changelog to the snapshot pins?
#1313560104359956542 message

would be added as a note under the main changelog, for example:
Changelog: https://www.minecraft.net/en-us/article/minecraft-snapshot-25w09a
-# Extended/Misode: https://misode.github.io/versions/?id=25w09a&tab=changelog

[frank sigil]

but they do use ADO internally (confirmed by looking at the new mojira)

[proven compass]

https://tenor.com/view/get-back-to-work-gif-10302248683148027648

microsoft:

[thin cairn]

Oh interesting

[eager mirage]

Are there issues preventing a neoform release?

This snapshot looks intense.

[gleaming pendant]

does anyone know what the point of all the gl abstraction is?

we have to patch DSA#bindFrameBufferTextures for stencil and I am not happy about it

[lunar saffron]

Yeah, I looked at that yesterday and very quickly threw it to the side 🤣

[gleaming pendant]

and CommandEncoder too... :/

(potentially)

[fleet dome]

Is KITS in a state where the patch c an be done?

if so, I'm up for it

[gleaming pendant]

you can fix the patch but you can't test it

[fleet dome]

Was a bit preoccupied with legacy C++ software I worked on late yesterday 😅

[fleet dome]

That's okay, I like yolo'ing to prod anyway

[gleaming pendant]

sounds good

then yes

we're having fun with models in case you haven't noticed yet 😛