1.20.2

[frail oriole]

no. modlauncher does that luke 😛

[mental carbon]

That issue needs to describe the situation,

[frail oriole]

ATs are in there too. you generally don't see them because they're good.

[mental carbon]

And should hold on to your idea of the compiler payload in the jar

[mossy bobcat]

Whoops, I forgot that we're still on the upstream mixin where it doesn't prefix method names...

[frail oriole]

and?

[mental carbon]

Cause IMHO, I don't think investigating this currently is worth our engineering time

[frail oriole]

i would agree orion

this has been a waste of two hours

[mossy bobcat]

We warn people that if they write mixins, it can cause those sorts of issues

[frail oriole]

well, not entirely, but now we're just spinning wheels

[mental carbon]

But we need to keep track of it

[mossy bobcat]

Should we be making the same warnings about coremods?

Sorry, ATs

Obviously yes for coremods

[frail oriole]

we already do for ATs? what do you want? big red flashing bars?

[mental carbon]

Technically you should hand these warnings out to everybody that indeed does at, core mods, mixins

[frail oriole]

hmmm

[mossy bobcat]

Because currently the understanding that the old forge spread to the modding community was that ATs were pretty safe

[frail oriole]

that wouldn't stand out on the start screen 😂

they're modifying bytecode. that comes with all the usual risks of modifying bytecode.

[mossy bobcat]

Yes but in the case of ATs its comparatively badly communicated, and the issues that can arise are a lot less obvious in terms of what can go wrong than with mixins

It's not worth warning AT users about, I would say, because yes, to a degree it is implied

[frail oriole]

i think the "ATs are ok" perception is another lexhangover

[mental carbon]

It has always been communicated that ATS modify bytecode

Hell they even modify source code

As the only system to do that!

So yss

[frail oriole]

ATs were the only ones that lm would generally not go into a fit about

[mental carbon]

All the rules that apply to bytecode modification apply to ats

[mental carbon]

Granted as long as you have srg at runtime, ats are pretty safe

[frail oriole]

that's an illusion, and one that we're now seeing be peeled away.

[mental carbon]

Well it is only peeled because of mojmap

[frail oriole]

mostly because people don't name methods m_12345_

[mental carbon]

True

Well I have decided tonight to replace Ng with ngnext in forge dev

At leats I try

[frail oriole]

cool

mojmaps makes that simpler i expect right?

[mental carbon]

Yeah

[frail oriole]

did we ever do the gradle cleanup or are you deferring to, well, ngnext 😄

[mental carbon]

Or is happening now

[frail oriole]

hehe 😄

[mental carbon]

Complete newv build script

[frail oriole]

nice

buildSrc overhaul too?

[mental carbon]

Yep

My goal is that by tomorrow evening to have patch generation

And by Sunday at least some form of userdev

[mossy bobcat]

https://github.com/neoforged/NeoGradle/issues/7 issue opened for posterity

[fallow sundial]

btw did anyone hook glms in?

[dull bison]

why should they not be hooked up?

[dull bison]

Reading the whole conversation above looks to me like moving to runtime mojmaps was rushed and not 100% thought through.
Can't we postpone that to 1.21 and do 1.20.2 with srg?

[mental carbon]

No

It was decided that 1.20.2 is the window for that

And it was not really rushed

We have been talking about this for months

We knew stuff like this could be a problem

But realistically it always has been a problem

[fallow sundial]

due to loot changes?

[tawny iron]

loot changes?

[fallow sundial]

yeah I don't see how glms would work now as they require the loot table to have its ID set

and since they're now decoded using the codec the forge hook that sets the id is never called

[dull bison]

ill have a look

[fallow sundial]

you'll probably need to add yet another param here https://github.com/neoforged/Kits/blob/1.20.2/projects/forge/src/main/java/net/minecraft/world/level/storage/loot/LootDataType.java#L40, something like biconsumer<T, RL>

[tranquil salmon]

#builds

[tranquil salmon]

1.20.2-rc1 with mojmap: #builds message

[lethal bane]

does runtime mojmaps not mean that technically ORH is no longer needed

unless of course people use something different than mojmaps in their dev env

[dull bison]

not just technically (it's gonna be nuked)

[fallow sundial]

yeeting any kind of remapping support for userdev runtime means that we do make yarn dev for example impossible

[dull bison]

it already would be afaiu

[mental carbon]

Given that the fabric devs are not even willing to consider common tags, I see no effective reason why we should support yarn at all

[dull bison]

modmuss is, player seems not to like it

[mental carbon]

If somebody wants to go the extra mile and add support for it, sure.

But I see no reason why we should support it natively

[fallow sundial]

why would what fabric does change the fact that yarn are independent fabric mappings and people use them?

(like rubidium/embeddium)

[dull bison]

they only use it because they are a lazy (copy paste) port from fabric to forge

[fallow sundial]

[dull bison]

using yarn (to me) seems wrong all together since we have the official mappings

[mental carbon]

IMHO us spending time to support their special environments is something we can do from our end, sure. However given that they can not even seem to think beyond their own loader and modders into the world of users, Datapacks and cross platform loaders shows to me that they have 0 interest in a better environment for all. So if that means us having a much simplified toolchain vs supporting yarn when doing reflection. Then the simplified toolchain wins

[fallow sundial]

sigh so if fabric only cares about themselves means it's completely fine for us to do the same?

[mental carbon]

No

[kindred fractal]

There is no reason to support yarn

[mental carbon]

I am just saying that when weighting the much much much simpler toolchain, in this particular case

It simply wins

[kindred fractal]

Also, don't confuse the opinion of Player and the opinion of Fabric as a whole

[mental carbon]

IMHO It is a gray area tech

The problem is

For things like compatibility

It needs changes on both ends

If only one team wants to make those, and with that their process more complicated

Then that is a significant detriment to that team

Cause 1) they did not get the hoped compatibility

And 2) they have now the much more complicated process

So if there is a single maintainer with veto power in any of the teams

And that causes the required changes to not be merged

That implicitly makes that the opinion of the other project

Whether that is true for all people involved is beyond the point

IMHO we should strave for as much runtime compatibility as possible

Which includes things like tags, resource pack formats, etc.

But that does not include making our toolchain completely compatible with theirs

[kindred fractal]

Forge doesn't need to support running on yarn

[mossy bobcat]

Can we make the official<->obfuscated names available at runtime somehow? That at least means that someone else has a better than zero chance of setting that up if they want to

[fallow sundial]

I mean didn't you complain about the same thing of "people taking what a pink person says as the 'opinion' of the project" in the context of neo? seems a bit hypocritical to me

player is free to have his opinion, but as it stands the PR wasn't closed by either party

[kindred fractal]

There's no need for accusations

Player being against the PR is not a good sign

[mental carbon]

It is a bit hypocrytical I agree. However I make a distinction between the community taking SC member opinions as the word of god. And the members of another loaders maintainance team making decisions which would make our live harder......... So it depends

[lethal bane]

You could just ship it yourself

although if srgutils goes away it's gonna be harder to parse the files

[mossy bobcat]

You'd have to download it at runtime, like people currently do

At least allowing access to that bit, which should be there anyways, means you'd only have to grab intermediary mappings, if that's what you're using, or whatever

[mental carbon]

We already have that

And we can feed the proguard file into it for sure

That is not a problem

[lethal bane]

then why remove ORH?

[mossy bobcat]

I only bring it up because you guys never used to do it with srg to obfuscated at runtime

[mossy bobcat]

Because the switch to Mojmaps let's you pull out a lot of the internal remapping wiring, I'd expect. Like, how would ORH even work? You'd give it Mojmaps names and get out Mojmaps names

[mental carbon]

Because ORH requires a massive amount of toolchain work

As well as an extremely legacy file format

There is no benefit to keeping it

Outside of the hypothetical yarn support

[mossy bobcat]

That said, I do think turning our back on the existence of other mappings, like yarn, isn't a good idea

[mental carbon]

I mean in essence we still support it, just not natively

Other runtimes are free to add custom tools

[mossy bobcat]

Have we talked with the arch loom people at all to see what the absolute minimum is for some sort of yarn support?

[mental carbon]

Like Arch already does

It has loom

And a runtime library

[mossy bobcat]

Arch look only exists at dev time though

[jagged agate]

Can documentation be provided that explains how to do other mappings?

[mossy bobcat]

Which many people using the library do not use

[mental carbon]

Well then, it becomes the question why not?

I mean sure we could add minimal support for it

But we would in essence need to keep the entire reobf chain

And all the processing in place

[mossy bobcat]

Because it's overkill 9 times out of 10?

[mental carbon]

To make custom mappings working

[mossy bobcat]

I suspect that on modern loom versions arch might be able to solve the problem by remapping to Mojmaps prior to in dev runs? I'm not actually sure though

[mental carbon]

I don't know

I mean in userdev land it does not really matter

[mossy bobcat]

That's why it's worth asking people instead of making assumptions and saying "up to them to figure it out"

[mental carbon]

The game and the mod are mapped to the same mappings anyway

So reflectively accessing the field/method is trivial

because the name matches

The thing ORH solved was runtime names

Because you needed to know the SRGId of the method at runtime

[mossy bobcat]

My point is that if everything is in Mojmaps at userdev runtime, then you just always use Mojmaps for reflection and it doesn't matter what userdev dev time is using

[mental carbon]

That first sentence is however not true

What is being argued is that yarn userdev should be possible

Which means both the game and the mod are then in yarn

[mossy bobcat]

Userdev runtime Orion

[mental carbon]

That does not matter

userdev runtime and userdev run with the same mappings

[mossy bobcat]

... #1136320550168436798 message must they?

That's my whole point here

[mental carbon]

For forge yes

They must

[mossy bobcat]

This is arch loom though

Not NG

[mental carbon]

What loom does I don't know

I mean if arch already has logic like that

[mossy bobcat]

Hence why I'm saying we should ask someone with more logic about what loom can do

Arch has no special logic for it

[mental carbon]

Then something that allows for example the name to be switched in a string constance using reflection is also an option

[mossy bobcat]

But loom may already make it possible, I'm not sure

[mental carbon]

We don't officially support running forge with loom

[mossy bobcat]

Yes, only if there was a way of targeting the right string constants, but yeah

[mental carbon]

What loom does now, is at the moment not really a concern of neoforge

Yes there might be na issue with orh

But the functioning of orh can be mimicked by toolchains

[mossy bobcat]

People use arch loom to develop neoforge mods, and will continue to do so, so it is a concern whether or not you offer official support

[mental carbon]

Ng just abuses the fact that userdev and play runtime are in the same mapping so it provides no service what so ever

But loom, arch loom or other variations are free to provide other services

[mental carbon]

Yeah but that also means that arch loom can and potentially should offer an orh like system, whether through annotations or in game runtime libraries

We are not saying it is completly blocked

[fallow sundial]

if orh is removed forge itself won't run on non-mojmaps

[mental carbon]

That is a given!

But that is not really up for debate

We already made the decision that forge runs everywhere on mojmaps

And that that is the only mapping we natively support

As in we.....

The Royal qe

We

But that does not mean that a toolchain can not make forge run on other mappings

Well

Mayve

I mean we can keep the ORH code

Patch it up to use SRGUtils

[mossy bobcat]

Arch look can't offer anything through in game runtime libraries... Arch and arch loom are unrelated systems. And yeah, I agree, the best solution here is for arch loom and company to have some dev time system for this. I'm just saying that maybe folks should, like, ask someone who knows arch loom better and see what pitfalls arise

[mental carbon]

But make it blind when we run in NGNext

The maintaince is minimal

[mental carbon]

The problem with the argument: "lets ask the loom folks what they can do", makes it seem like the decision to switch to mojmaps as the sole mapping channel, depends on what they say. When they have 0 say in it. Yes they can provide some inside. But no, there capability to change something is not really on table I am afraid

Like the best we can do is offer them to keep the ORH infra around

For them to fill at runtime

But nothing more

[mossy bobcat]

That's a non sequitur. Letting them know "hey this is happening, anything we should be aware of that we don't already know" does not imply that at all...

[mental carbon]

Yeah but what do you then do with the answer luke?

What would the concequence be of them saying: "Well we can not support that then"

[mossy bobcat]

You... Have more information now?

[mental carbon]

Would that information change anything?

[mossy bobcat]

You know if there's systems neoforge can expose that are minimally invasive and make their lives easier?

[mental carbon]

I already offered to keep ORH around

[mossy bobcat]

Who knows if that information would change anything? We don't have it yet, do we

[mental carbon]

But I can't really do anything more

[dim sonnet]

What about runtime remapping?

[mental carbon]

That would require an arch runtime module

Which luke says they won't provide, or modders won't use

[mossy bobcat]

Runtime remapping never works anyways

Or rather, it'd have to be remapping from intermediary

[dim sonnet]

How'd you think srg works?

[mossy bobcat]

In which case you might as well use Mojmaps instead of intermediary

[mossy bobcat]

ORH in production doesn't do anything though

It just returns back the same thing it got

It only does stuff in dev

[dim sonnet]

I think you're confusing mojmap and srg

Forge is mojmap in dev, srg in production at runtime

[mossy bobcat]

I'm not. I'm using Mojmaps when talking about 1.20.2 in place of srg

[mossy bobcat]

Yes. ORH is fed srg names. In dev, it spits out Mojmaps (or yarn or whatever you manage to set it up with) names

In production, it doesn't remap at all

[dim sonnet]

So if we have already achieved obf->srg in production for a long time now without remapping problems, we could also do it for srg->mojmap

[mental carbon]

IMHO we should just keep the orh code and flags for now

[mossy bobcat]

I'm not super worried about this being an issue, honestly. With my rudimentary knowledge of loom I think it is probably solvable on that end with some cleverness. That said, I am much more worried about the approach that you guys seem to think is reasonable for this - that is, making large changes without specifically reaching out to collect information from groups known to be affected majorly by those changes. That information need not change anything, but there's a huge importance, both symbolically and in terms of just finding out stuff you might have forgotten about

[mossy bobcat]

There is no obf->srg remapping at runtime

[dim sonnet]

In production we're not using obf. It gets remapped to srg at runtime (or install time)

[mossy bobcat]

That happens during installation

Yes. Install time and runtime have access to completely different bits of stuff

[dim sonnet]

Part of the plan for neoforge was moving that to runtime anyway

[mossy bobcat]

It's why you can't easily get at the obf-srg mappings at runtime currently

[dim sonnet]

Having an extra mode that supports srg->moj for mod code shouldn't be too hard once the infrastructure is in place for doing vanilla obf->moj at runtime

[mossy bobcat]

Then you need to have srg at runtime

[dim sonnet]

But you continue supporting other mappings seamlessly

[mossy bobcat]

But sure. I'm not actually worried about that as I figure that sort of stuff will work out as changes are made

[mossy bobcat]

You can do that with Mojmaps too? Other mappings should be supported by converting to Mojmaps - Mojmaps becomes your intermediary mappings

[mental carbon]

IMHO is the toolchain supports other mappings at runtime

Then it should take care of that

[dim sonnet]

Then your argument that we need to ask others about it before switching becomes pointless. Because you also argue that they can adapt so it doesn't matter

[jagged agate]

cc: @thorn robin on this convo. Tldr: ObfuscatedRemappingHelper or whatever will be removed and Forge uses Mojmap at runtime. Will Arch struggle with this?

[mossy bobcat]

I have no clue where you got that from

[mossy bobcat]

Ah, good, wasn't sure who was in this server from arch

[mental carbon]

Thanks I did not know they are in here...

[dim sonnet]

You say here that other mappings can be supported by supporting the new intermediary in response to my proposed solution for not needing to do that: #1136320550168436798 message
And around here you're wanting Orion to ask others before deciding on switching to mojmap intermediaries: #1136320550168436798 message

[lethal bane]

runtime mojmaps would mean that all AT files have to change, right?

[mossy bobcat]

Switching intermediaries is a pretty minor thing that is by definition easy to adapt to. That is not the only change happening here, which is why you should definitely be at least talking to other people

[dim sonnet]

No, because iirc FG6 supports mapped ATs already

[mossy bobcat]

This is "let's get rid of the whole concept of reobf" not just "let's switch intermediary mappings"

[mossy bobcat]

In practice yes, because remapped ATs have issues in FG still so aren't actually used

ATs have to change, mixins won't need refmaps (or will generate empty ones)

[fallow sundial]

it doesn't

[dim sonnet]

What issues?

[mossy bobcat]

Something something eclipse I thought?

Could be wrong

[fallow sundial]

mapped ATs were failing randomly on CI so they never became part of fg

[mossy bobcat]

NG will still support reobf, right? It'll have to

Since otherwise how would you use it for old MC versions?

Like, unless you're throwing out the idea of retrograde entirely

It just won't use reobf for most mods

[dim sonnet]

NeoForged have said many times they're not really interested in older versions anyway atm

[mossy bobcat]

At the moment no but that doesn't mean "throw retrograde out the window entirely"

[dim sonnet]

RetroGradle's been in dev for over 2yrs and still hasn't done one version... I think it's low priority

[mossy bobcat]

Didn't they get 1.12 working, at least to a reasonable level of working?

[dim sonnet]

That was lex. He did it in a week or two

RetroGradle's trying the more involved approach of writing automation scripts and rewriting a decompiler and stuff

Su5eD did 1.4.7 in a couple of months iirc

[mossy bobcat]

Still seems like something the door should be left open to

[mental carbon]

No

Why does it need to?

[mossy bobcat]

Using it with old MC versions?

[mental carbon]

Use an old NG version

[mossy bobcat]

New NG will be unusable with anything that isn't 1.20.2+ then? ...alright then.

[mental carbon]

You are using an old java, old forge, old libraries and dependencies

So why does the new ngnext version need to be able to support you al the way back to like what 1.7.10?

[mossy bobcat]

...I never said that

[mental carbon]

IMHO it depends

I mean I am willing to make something like a "legacy" package, which technically already exists

[mossy bobcat]

Stop making straw man arguments out of what I said. If you don't support reobf, you don't support 1.20.1, even ignoring earlier stuff

[mental carbon]

That at least can add support for reobfing back

[mossy bobcat]

That would make sense to me

[mental carbon]

But initially it won't

[lean warren]

Where did you get that from? There was never a plan to move obj->srg/moj remapping from the installer to the runtime. Ancient Forge did that and it was slow as balls

[kindred fjord]

Don't remember the details, but wasn't it slow on ancient Forge because it remapped on every launch and didn't cache it or something like that?

[stray scroll]

gotta love on-the-fly runtime remapping

[dim sonnet]

I can't remember if it was an internal plan to announce for nap or publicly stated so I'll be quiet now

[lean warren]

Yeah, that's exactly what it did

[mossy bobcat]

No reason neoforge couldn't do it on the first launch

Whether that's a change worth making is the question

[fallow sundial]

there have been talks about remapping mods from 1.20.1 forge srg at runtime

because there's going to be mods that haven't been broken by 1.20.2

[jagged agate]

1.20.2 is when the registry refactor is right?

[fallow sundial]

supposedly

[mossy bobcat]

Hopefully

Reasonably though I think this is as good a time as ever to just, like, do a hard split and change the mod ID (and not provide the forge mod ID) and all that. Mods will update

[thorn robin]

probably leads to more hardcoding (etc mixins without refmaps, reflection directly)

can’t think of much else on top of my head

will probably be annoying supporting yarn, but we will need to see

would like to know if there are other toolchain changes in place

[mental carbon]

I can keep you up to date for sure

[thorn robin]

thanks

[frail oriole]

Lot of frustration in here this morning.

Can we lower the temperature a bit

Remember: the FIRST goal is to get something together for 20.2

And, because we don't want to have a competing definition of SRG, we have chosen to use runtime mojmaps

THAT is key. Having two competing definitions of SRG is infinitely worse than anything else discussed here

The fact that runtime mojmaps simplifies a lot of our toolchain is a side benefit.

[mossy bobcat]

I think everyone involved agrees that runtime Mojmaps is a net benefit that we should definitely be using

[frail oriole]

I'm hesitant to throw out all vestiges of the name mapping system just because we are getting this simplification for ourselves, but it will have to change

So why are there so many people throwing up roadblocks?

20.2 is the ONLY time we can do this

We can't defer because double srg

Lex isn't going to change his toolchain

We are the movers, we have to do the change

[mossy bobcat]

It's not "throwing up roadblocks" but "pointing out side effects". Like, yes we need to be switching to Mojmaps - but we should be aware of the consequences, and have ideas about eventual mitigation. Aware is key there - it's not going to delay switching to Mojmaps, but I might say we should delay, say, tossing out ORH

[frail oriole]

Then why does everything read like a roadblock

[mossy bobcat]

And you should have a plan for NeoGradle to still support reobf, even if right when 1.20.2 releases it doesn't

[frail oriole]

I just spent 20 minutes reading backlog and it seems clear you don't want to do mojmaps

Except you do, obviously

[mossy bobcat]

Well, mostly because someone involved likes to exaggerate what is said and make a bunch of straw man arguments, which turns "pointing out a side effect" into "road block"

[mental carbon]

I don't really exaggerate, and you can use my name

I am not made of glass

I am just pointing out concequences of your ideas

[mossy bobcat]

And also because whenever I say "I have issues with how you're doing this" certain people read that as "I have issues with the fact that you're doing this"

[frail oriole]

Luke, chill a bit please. There's dozens of issues with switching to mojmaps, and speculative guessing isn't super helpful when we're in the middle of the development work

Let's do the work and start trying to figure out what the net impact actually is.

The work has to happen anyway

[mossy bobcat]

I am chill, but I'm also tired of Orion continuously exaggerating everything I say. It happened yesterday with that issue, it happened today with this

[frail oriole]

We cannot NOT do this

You don't read as particularly chill, sadly

[mossy bobcat]

I agree! And that means reaching out to those likely to be affected. Which I got pushback for even suggesting

[frail oriole]

The response was as I expected. We'll see once we have done the work.

[mental carbon]

^

[frail oriole]

Those affected are here. And so far they don't, probably rightly, have much opinion yet, because they have nothing concrete to look at

[mossy bobcat]

Is there an actual reason we have to rip out remapping stuff from NG/neoforge at the same time as switching to Mojmaps?

[mental carbon]

Yes and no

Yes in the sense that we need to rework many internal components anyway

[frail oriole]

I mean, it's not really ripping out. It's adding it to ngnext right?

[mental carbon]

and as such not having to rework those that we do not need anymore is a major plus

But no

Technically there is no reason

[frail oriole]

Other than the large extra work effort

[kindred fractal]

I don't see why any complexity needs to be kept for people using yarn, really

[frail oriole]

Let's do the needed work first.

[kindred fractal]

If they want to keep going against the recommendation of using mojmap they can keep finding workarounds, that's on them

[frail oriole]

Then we can worry about the bells and whistles later

[mental carbon]

Sorry luke If I came on strong

But I am trying to keep the port on track

And that means sometimes

I can not involve every party in every discussion

Because we could not get anything done

[frail oriole]

Yeah. I want to make sure that orion can focus first and foremost on 20.2

[mental carbon]

So if I ask questions, as to what the expected results of your ideas are

I am just trying to determine if it is worth it

[frail oriole]

And <waves at channel> is not great as a distraction from that

[mental carbon]

Versus the time it takes

[mossy bobcat]

That's fine and I understand that Orion, I just wish you would please, please stop exaggerating statements I make to the degree that has happened during this conversation and the last one

[mental carbon]

I don't see how I am exaggerating

[frail oriole]

Luke, catastrophism is a thing in this crazy world.

[mental carbon]

I am just pointing out concequences

They can be on the fringe side

But we both know that MC modders tend to go to that fringe side real quick if we are not carefull

[frail oriole]

Yes

[mental carbon]

So I tend to consider the extremes more thorougly

Then the soft fluffy middle

[mossy bobcat]

I say "NG needs to support reobf for old versions". You say "luke wants NG to support 1.7.10 and that's your issue". I say "Mojmaps and ATs can lead to this issue that is hard to debug and we need to prevent that issue from happening". You say "luke wants us to delay Mojmaps"

[mental carbon]

I wanted to know your reasoning for keeping it?

[mossy bobcat]

There've been a lot of straw man arguments going on here and it gets tiring

[mossy bobcat]

I dunno, how about 1.20.1?

[frail oriole]

It gets tiring to read it all too luke

[mental carbon]

Why?, Like what is the differnce between 1.20.1 and 1.7.10

[frail oriole]

Why are you focusing on 20.1. The problem isn't 20.1 it's 20.2

[mental carbon]

Supporting one means supporting the other

Because the toolchain is mostly the same and covered by the same functionality

[frail oriole]

This channel is about 20.2

[mental carbon]

So if we are about to make a hard cut

[frail oriole]

And oNLY 20.2

[mossy bobcat]

Currently, NeoGradle can be used on most relatively modern MC versions without issue. If the reobf tooling is pulled out entirely, this is no longer the case

[true pivot]

Which is not a huge loss IMO

The old versions will still work

[frail oriole]

Yup!

[mossy bobcat]

That is a relatively large change to what NeoGradle attempts to support, compared to what forgegradle did. Has there been some sort of vote on that? It seems like something that would require more discussion than just "easiest to rip it all out so..." Which is all I've really seen on it

And I've not really seen any of that discussion in public channels

[true pivot]

Who said the legacy branch would be completely unmaintained?

[mental carbon]

^

And we are not removing older version from the maven

Or the MDKs

[frail oriole]

Luke, you understand that that point is exactly what I was saying a minute ago, right?

[mossy bobcat]

I do understand. I was replying to Orion's question

[frail oriole]

Nothing is going away. We're not going to retroport the removal to all other versions.

You know Fg6 only works on 19 and 20 right?

[true pivot]

Wait really?

[mental carbon]

Yeah

[frail oriole]

That versions bump and change capabilities all the time, right?

[mental carbon]

But FG5 works on both

As long as you use an older gradle version

[frail oriole]

Yeah. But my point is that ng removing remap for 20.2 isn't a calamitous disaster

[mossy bobcat]

That seems... Problematic. You lock anyone on old MC versions into old gradle versions. Those old gradle versions eventually run into issues - heck, look at why retrogradle has to exist at all - and it seems like it's much easier to work on keeping some degree of support for it as you go along, than it would be trying to fix stuff when it does break

[frail oriole]

That hasn't even happened for Fg6 luke

And doing that work effort gets exponentially more difficult

[mossy bobcat]

Can we modularize stuff? Shove the legacy stuff somewhere separate from the modern stuff and just make sure it has a way to interface with everything?

[frail oriole]

Not without a lot of work.

Maybe we can do it. Is it anything remotely close to the top 10,000,000 items on our list? No

[mental carbon]

I also already suggested expanding the Legacy module of NGNext

With those kinds of capabilities

[frail oriole]

Please don't waste time on it right now tho

[mental carbon]

No

For sure not

[true pivot]

Wanted to respond to this - I find this to be a pretty bad faith argument - as an example, using the same mappings as the upstream project is just common sense if I plan on upstreaming anything. That doesn't mean the native Forge toolchain needs to support it, but it's not an invalid use case

[mental carbon]

I have NGNext booting in Forge at the moment

😄

[frail oriole]

Yay!

[mental carbon]

It is making the clean project currently only

And the neoform is next

[frail oriole]

Well,close!

[mental carbon]

But I got it too load 😄

[mossy bobcat]

Having a plan of "yeah, we'll eventually offer a legacy system with that capability on NGNext" is all I was asking for. I just didn't appreciate having my statements drastically exaggerated

[true pivot]

People kept FG1 working well past its best before date - I think this is less of an issue than you think

[frail oriole]

Like, I don't want to commit people to work, especially work that's probably of low value with little need

[true pivot]

And that was without any upstream cooperation to get anything fixed officially

[mossy bobcat]

Oh sure. But leaving the door open as a "this is a good idea and we'd like to eventually get to it" doesn't do that

[frail oriole]

And you should understand that

It doesn't read like you do

[mossy bobcat]

I do understand that... When have I said that I don't

[true pivot]

Your statements (to me) read as if you expect Neo to guarantee they will fix this at some point

[frail oriole]

What I'm going to suggest, is we separate this topic out

[mossy bobcat]

Orion and company seemed to be implying that it was a good idea to rip out these remapping systems entirely, with no plans to ever put them back in. That's what I was pushing back against

[frail oriole]

Make a new brainstorm about the potential to support other mappings and legacy stuff in ngnext

Well Luke that's how we're going to start

[mossy bobcat]

...at which point I got rather annoyed, as my statements got exaggerated to "you want NG to support 1.7.10" which is a whole different bucket of worms

[true pivot]

I still think this is fine. It's unnecessary tech debt

[mental carbon]

I never said I would not put them back, hell we even have a legacy module already in ngnext, and I already stated that keeping ORH, and expanding Legacy is an option

[frail oriole]

Because if you don't you have to fix them all to compile which is a huge waste of effort

Anyway can we move this to a different brainstorm please

This should be about 20.2 first and foremost

<@&1103212714219802654> can we make a new brainstorm for legacy discussions post 20.2

Out for a few hours now

[formal plume]

just a legacy pre 1.20.2?

[mental carbon]

Yep

[formal plume]

there you go

[tranquil salmon]

In case anyone asks about a neoform version for rc1 #builds message

[lethal bane]

are there tools planned to remap AT files?
I personally wouldn't need them, but some have a lot of entries

[mossy bobcat]

You mean when converting 1.20.1 to 1.20.2?

It should definitely be possible though I'm unsure how complicated it could get

[lethal bane]

yeah, since all entries atm are in srg and that won't work once runtime mojmaps exist, aiui at least

[fallow sundial]

that doesn't make sense?

fg6 should definitely work on 1.18 and 1.16

[prime quest]

We removed older gradle version hacks in fg6

It doesn't work on older gradle

Because we figured it's getting way too painful to maintain a toolchain that works on a vast array of very disjoint gradle versions

[fallow sundial]

and that's relevant to it "only working" on 1.19 and 1.20 how?

and in any case

the "hack" (artifactural) wasn't removed it's still there https://github.com/neoforged/Artifactural/commit/1d671ed9f1aad8cc68f18afda47febdddd60f0d8

and fg6 was only working on g8+ anyways

[jagged agate]

latest is greatest. yeet older support

[prime quest]

fg6 only working on g8+ is what I mean

We stopped adding hacks to gradle internals in FG itself to support lower gradle versions

Separately from Artifactural

Not working on 1.19/1.20 I guess is what cpw confused for dropping support for older versions, except we meant for gradle rather than for Minecraft

[fallow sundial]

fg only works on one gradle major ever since 5 anyways

it's definitely not a new thing :P

[prime quest]

Yes that's exactly my point lol

[mental carbon]

Setup in parallel 😄

[prime quest]

inb4 people complain that we start using more of the CPU

[mental carbon]

I need to fix the damn logging of the naming license

[fallow sundial]

9696 warnings

mhh

efficiency

[mental carbon]

They are the damn file headers now in the mapping file

[fallow sundial]

quality code has more warnings than LoC

[mossy bobcat]

As long as it's easy enough to disable... looking at loom that crashes the IDE whenever I build unless I disable parallel stuff

[true pivot]

awesome

[mossy bobcat]

I was gonna say, I was pretty sure fg6 worked on older stuff

but this is the wrong channel for that discussion

[tranquil salmon]

Did anyone notice that mojmaps for each class now includes the file name of the source file

[mental carbon]

YEP

Those where the 4500+ warnings I just had in that image

Because I just did a filter on #

To get the license lines

[fallow sundial]

just why

[mental carbon]

No clue

[true pivot]

what functional purpose does that serve - I thought the source file name is basically given by the classname

[fallow sundial]

inners?

[dull bison]

I have the fix for GLM ready should I do it on the old pre1 state or on the pre4 state?

[mental carbon]

No pre4

We are not going back

It gets problematic

[dull bison]

so I should wait on your mojmap stuff for rc1

[mental carbon]

No go ahead

[dull bison]

so on current state (which is pre4)

[mental carbon]

Yep

Simply continue along

I hope to have patch apply working tonight

Then compile

And then genPatches

Before progressing towards runs / installers etc

[fallow sundial]

hmm, we should probably set the names of loot pools to a fallback index

[mossy bobcat]

Does it not do that already?

[fallow sundial]

not in pre4

[mossy bobcat]

Ah, makes sense

[dull bison]

but how would you get the index? it is a codec

[mossy bobcat]

Can you dump the codec here so I can look at it?

Or is it literally just something like LOOT_POOL_CODEC.listOf()

[dull bison]

public static final Codec<LootTable> CODEC = RecordCodecBuilder.create(
   p_297999_ -> p_297999_.group(
            LootContextParamSets.CODEC.optionalFieldOf("type", DEFAULT_PARAM_SET).forGetter(p_298001_ -> p_298001_.paramSet),
            ExtraCodecs.strictOptionalField(ResourceLocation.CODEC, "random_sequence").forGetter(p_297998_ -> p_297998_.randomSequence),
            ExtraCodecs.strictOptionalField(net.minecraftforge.common.crafting.conditions.ConditionalOps.decodeListWithElementConditions(LootPool.CODEC, "forge:conditions"), "pools", List.of()).forGetter(p_298002_ -> p_298002_.pools),
            ExtraCodecs.strictOptionalField(net.minecraftforge.common.crafting.conditions.ConditionalOps.decodeListWithElementConditions(LootItemFunctions.CODEC, "forge:conditions"), "functions", List.of()).forGetter(p_298000_ -> p_298000_.functions)
         )
         .apply(p_297999_, LootTable::new)
);
public static final Codec<LootPool> CODEC = RecordCodecBuilder.create(
   p_297996_ -> p_297996_.group(
            LootPoolEntries.CODEC.listOf().fieldOf("entries").forGetter(p_297995_ -> p_297995_.entries),
            ExtraCodecs.strictOptionalField(LootItemConditions.CODEC.listOf(), "conditions", List.of()).forGetter(p_297992_ -> p_297992_.conditions),
            ExtraCodecs.strictOptionalField(LootItemFunctions.CODEC.listOf(), "functions", List.of()).forGetter(p_297994_ -> p_297994_.functions),
            NumberProviders.CODEC.fieldOf("rolls").forGetter(p_297993_ -> p_297993_.rolls),
            NumberProviders.CODEC.fieldOf("bonus_rolls").orElse(ConstantValue.exactly(0.0F)).forGetter(p_297997_ -> p_297997_.bonusRolls),
            Codec.STRING.optionalFieldOf("name").forGetter(pool -> java.util.Optional.ofNullable(pool.name).filter(name -> !name.startsWith("custom#")))
         )
         .apply(p_297996_, LootPool::new)
);

[mossy bobcat]

And we want to give each loot pool a name based on its index unless it has a name field, right?

[fallow sundial]

in the ctor

[mossy bobcat]

Yes, getting the index there is the painful bit

Okay, so, you should be able to make a codec that decodes the list to a List<Pair<Integer, LootPool>>. You have to make sure that this is done within the condition stuff, as the index there should be index before conditions are applied (changing conditions shouldn't change the index!)

Then, you xmap it to make new loot pools with the generated names if they don't have them already

Unfortunately, this seems like it could complicate your otherwise quite clean condition stuff

But the index has to be passed before conditions are applied, otherwise you end up with issues

[fallow sundial]

sounds like time for a custom codec tbh

[mossy bobcat]

Yes that is involved in that process I mentioned

My solution is with a custom codec

[fallow sundial]

abstracting that out won't be nice

[mossy bobcat]

This is... Probably impossible without a custom codec

[fallow sundial]

I know, I meant a hard-coded custom codec

:p

[mossy bobcat]

The biggest issue here is that you have to sort of split up what you're doing all-at-once in your condition stuff

[quiet talon]

Can't you just post-process the names

[mossy bobcat]

Because the built in list codec doesn't track indices

[quiet talon]

Just loop over the deserialized result

[mossy bobcat]

No, you need pre-condition indices

[mental carbon]

IMHO, if the table has no index

[mossy bobcat]

Otherwise an early condition changing mutates names for everything after it in the list

[mental carbon]

has no name*

Then it is not targetable by GLM

Cause else how would this work

Or is there currently a way to deal with the GLM via indexes?

[mossy bobcat]

You generate a default name based on the index, yes

That's what happens pre 1.20.2

[mental carbon]

But how do you target that name then?

Cause it seems to be determined at runtime at the moment

Is it not?

[mossy bobcat]

You target the name?

It's determined fully from the JSON structure

[mental carbon]

Yeah but you do not know the index ahead of time

[dull bison]

currently if it doesn't have a name it gets a default name "custom#<index>"

[mental carbon]

Or is it specific to the loottable file

[mossy bobcat]

As much as you know the name, you do, yes

Its index within that table

[mental carbon]

Ahhhh

[mossy bobcat]

Not some global index

[mental carbon]

Well that can become very difficult to implement

Especially with codecs.....

But even more so when it has to be unique across conditions

Cause post processing is then out of question

[mossy bobcat]

Nah, not too bad. You'll need an equivalent of listOf that goes to a pair with the index

And an equivalent version of that for your condition stuff, like you already do with listOf

And call it a day

If you gave me ~1.5 hours I would be free and could probably write it if nobody else gets to it

(that's the abstracted version. If you just want a one off it's a single custom codec, as maty said)

[fallow sundial]

just give me a second, I'll do it

[dull bison]

it's been 7 minutes

[fallow sundial]

yes I need to run setup

[mental carbon]

Forge now running as well

[dull bison]

we have pre4 running

[fallow sundial]

did you install the fps nerf mod

/s

[mental carbon]

Very nice

Now we sadyl need to wait

Untill I get the patch generation in NGNext working

[fallow sundial]

hmm are there stream collectors that allow for null vallues

ah found ImmutableList.toImmutableList() from guava

[dim sonnet]

Or simply .toList()

[fallow sundial]

ah yeah that works

[dim sonnet]

If you have to resort to guava when using Java 17, you're probably doing something wrong 😛

[fallow sundial]

I meeean

guava's looks more efficient :p

the default jdk one is a wrapper of a wrapper of an array

[prime quest]

I mean, streams are atrociously inefficient in the first place

You're saving cycle on seconds

[fallow sundial]

best method name

[dull bison]

please split that and use one as the parameter for the other

[fallow sundial]

no, the point of it is to do both, so the patch isn't ugly

it's better if the lambda soup is somewhere else so the patch looks like this

[frail oriole]

afternoon. i'm going to cry in my lamdba soup. 😭

[mossy bobcat]

If you're already using streams it's probably worth it just to filter out the nulls, right? Unless you mean that you want to include them in the collection

[fallow sundial]

you have a point, Optional#stream will never give a stream with a null value, will it...

[frail oriole]

no

[fallow sundial]

I might just not use a stream tbh, it's probably better if i just use an immutablelist builder

[mossy bobcat]

Can also just do orElse null and a filter to check if it's null

But yeah, if you're really worried about efficiency go with a builder

[fallow sundial]

better

[mossy bobcat]

... why are Java streams so inefficient? What's it actually doing that takes so long?

[frail oriole]

they're not

java streams are pretty efficient. the expensive part is typically the object allocations - you can get into a state where you're building dozens of objects to support iteration, for example. that's where the inefficiency comes from.

[mossy bobcat]

Ah, that checks out

[frail oriole]

i've got quite a few tests that literally demonstrate this. the eventbus stuff, and the modloader stuff both use streams and don't suffer from horrendous performance in general

[mossy bobcat]

I can imagine, like, something that involves a flatMap would make a lot of objects

[frail oriole]

not really?

it's very case specific. and there's a lot of optimizations (and more for newer jdks!) that try and make sure they don't vary from the normal iterator loops

[mossy bobcat]

Makes sense to me

[dull bison]

maty your "fix" will error

you always set the name but you should only do that if it doen't have one

[dim sonnet]

Read the javadocs and check all implementations... It usually ends up using a jit sharedsecrets shortcut

[lethal bane]

but they're nicer

[dim sonnet]

The one you're referring to even specifically mentions in its javadoc that it's a fallback and much more optimised and specialised variants are usually used instead

[lean warren]

Streams are great but sometimes you must resist the urge. This loop for example is measurably faster (as in tens of milliseconds faster) than the same thing with a stream terminated by a reduce operation when running in the code path that generates almost 4000 semi-complex block collision boxes: https://github.com/XFactHD/FramedBlocks/blob/1.20/src/main/java/xfacthd/framedblocks/api/shapes/ShapeUtils.java#L20-L27

[lethal bane]

[mossy bobcat]

Ah, VoxelShapes. Making stuff really fucking slow since... I actually don't know when

[lethal bane]

Still wished this was the case #squirrels-🦊 message

[mossy bobcat]

I bet kotlin lets you do that

[fallow sundial]

...good point

[sly anvil]

On a perfect day when the JIT works perfectly and everything gets inlined and specialised it should. It's just it's not reliable in practice :(

[fallow sundial]

what happened here

[inland mesa]

srg to mojmap

[mental carbon]

Okey

My initial work is basically complete

I now need to create a working version of the source code and patches on MojMaps

[fallow sundial]

it's not an inheritance thing which is why i'm confused

[mental carbon]

@dull bison The current working branch right

That is on SRG still?

Correct

[inland mesa]

I have no idea why it happened tbh, I just stated the symptom

:P

[dull bison]

the current state on the 1.20.2 branch is srg and working

if maty didn't break anything

[fallow sundial]

I just had a very bad idea

[fallow sundial]

trying to find a way to allow pool-level conditions in datagen is tricky

[quiet talon]

Just make the builder hold a pair<condition, pool> or smth

[fallow sundial]

yeah that's not the bad part, putting the condition inside the loottable and wiring it into the codec is the fun-er part

I could technically replace the pools list with a list<withconditions<lootpool>> but that's not going to be very efficient

[mossy bobcat]

Have people provide a special LootTableWithConditions instead of a LootTable

[fallow sundial]

that doesn't help, I'd need to basically copy and change the codec

[mossy bobcat]

Yeah, that's basically your only option

Given that the codecs are embedded within the structure, not top level

[fallow sundial]

this is where a thing like this would help

[mossy bobcat]

Wait wait wait I'm stupid

How does a Codec.pair write stuff?

It just writes the first thing it gets probably, right?

Except that wouldn't really work would it... Meaning it writes both and overwrites stuff

Alright I know how to do this then. Let me get out a laptop so I can write it up

[mossy bobcat]

Yep, as I thought

Second then first? Weird. Important here though I suspect

[fallow sundial]

the biggest problem here is the nesting, like I need to overwrite the mapcodec that is part of the top-level codec

[mossy bobcat]

Yeah I can do it. Give me a sec

[fallow sundial]

the other cursed idea I had that is very very inefficient

mapResult on the top-level codec, build the recordbuilder, get the pools field, modify each one of the elements of the pools array to inject the conditions of the pool, and then add it back

[mossy bobcat]

I can't see the private repo. @fallow sundial is there a way to take a Codec<A> and turn it into a Codec<Pair<Condition, A>> easily?

I'm assuming there is given how conditions work

[fallow sundial]

there's a conditionalops that gives you a WithCondition(List<Condition>, A)

[mossy bobcat]

Can you show me what datagen for something with a condition at the root level looks like?

You provide a condition and an object I'm assuming, right? I just want to see what one of those codecs looks like

[mossy bobcat]

Or does it always require a list of conditions?

[fallow sundial]

    final var codec = ConditionalOps.createConditionalCodecWithConditions(Codec.STRING.xmap(Thing::new, Thing::str)).codec();
    codec.encodeStart(JsonOps.INSTANCE, Optional.of(new WithConditions<>(List.of(new ModLoadedCondition("abc")), new Thing("ababa"))))
            .result().orElseThrow();

[mossy bobcat]

What's the type of the codec?

[fallow sundial]

Optional<WithConditions<A>>

there's another variant that xmaps that into discarding the conditions

[mossy bobcat]

How do I get the thing and the conditions out of WithConditions?

[fallow sundial]

but if i replace the list<lootpool> with list<withconditions<lootpool>> the conditions end up being kept around forever which is not necessarily a good idea for memory consumption

[fallow sundial]

public record WithConditions<A>(List<ICondition> conditions, A carrier)

[mossy bobcat]

Yeah I've got a way around it I just needed to know names

@fallow sundial how's something like this? (probably has errors as no IDE was used):```java
record ConditionalTable(LootTable table, List<List<ICondition>> poolConditions) {}

Codec<ConditionalTable> CONDITIONAL_CODEC = Codec.pair(ConditionalOps.createConditionalCodecWithConditions(LootPool.CODEC).listOf().fieldOf("pools").codec(), LootTable.CODEC).flatXmap(p -> DataResult.success(new Pair<>(p.getSecond(), p.getFirst().stream().map(WithConditions::conditions).toList())), p -> {
var pools = p.getFirst().pools;
if (pools.length() != p.getSecond().length()) {
return DataResult.error(() -> "Differing number of pools and coditions provided")
}
List<WithConditions<LootPool>> list = new ArrayList();
for (int i = 0; i < pools.length(); i++) {
list.add(new WithConditions<>(p.getSecond().get(i), pools.get(i)));
}
return DataResult.success(new Pair<>(list, p.getFirst()));
}).xmap(p -> new ConditionalTable(p.getFirst(), p.getSecond()), t -> new Pair<>(t.table(), t.poolConditions()));

Oh wait I need to handle the optional bit

Erm, is there a tool to create a Codec<List<WithConditions<A>>> or will I have to flatMap?

Anyways, replace createConditionalCodecWithConditions(LootPool.CODEC).listOf() with something that makes a Codec<List<WithConditions<LootPool>>> and you're good to go I believe

Possibly with a few tweaks needed

Wait I screwed up again... let me fix that

This is really hard without an IDE

But yeah, general idea is to write the whole loot table, then overwrite the pools with a version that has conditions

Could warp it around a bit to let people target pools based on names - that sort of magic happens in that big flatMap where you're fully aware of the pools and the conditions of the thing you're encoding

[dull bison]

orion how are runtime mojmaps going?

[mental carbon]

Slow

I have a problem with patch apply that I can't figure out

We can hop in a call in 20

[mental carbon]

Okey with the help of many people: ngnext is currently roundtripping forgedev

[frail oriole]

ok. next up, i'm going to look at getting the ATs working properly again, because we have some visibility changes in the patches RN

[mental carbon]

Those might be normal......

Double check if they are applied first 😄

But you already knew that 😄

So carry on 😛

[frail oriole]

ye 😄

next/neo should contain the patcher stuff, right?

nevermind it does

[mental carbon]

Yeah it is known as Platform

[frail oriole]

hmmm

[mental carbon]

That is not an error I am getting

Let me check

[frail oriole]

i have nothing in that directory

this is a checkout of kits, with clean run. then :forge:setup

[mental carbon]

Pull ngnext

I updated the setup task to treat those as outputs

[frail oriole]

BUILD SUCCESSFUL

[mental carbon]

Noice 😄

I am currently looking into bin patches

We are actually genning them for each different distribution type

I did not know that

[frail oriole]

LOL

public net.minecraft.advancements.CriteriaTriggers register(Ljava/lang/String;Lnet/minecraft/advancements/CriterionTrigger;)Lnet/minecraft/advancements/CriterionTrigger; # register

-   public static <T extends CriterionTrigger<?>> T register(String p_300853_, T p_10596_) {
+   private static <T extends CriterionTrigger<?>> T register(String p_300853_, T p_10596_) {

we're undoing the AT 😂

[mental carbon]

Yeah I noticed that in the old system as well

[frail oriole]

that's whatll happen if you patch in an AT

i'm trying to fix it, gimme a minute.

[dull bison]

I have a rebased version of the ngnext branch should I push that to 1.20.2 or force push it to 1.20.2-ngnext

[frail oriole]

wait please

[mental carbon]

Don't

I am about 95% sure that the rebase you just did failed

Or has broken patches

Or any number of issues

If you push anything, then to a complete new branch

So that we can vet it

I trust that you did it right

But maty also pushed broken code

And I just want to be double sure

Before I have to do all the patch fixing for the fourth time

[frail oriole]

yes

i'm currently neck deep in patch fixups for AT

this is not concurrently workable

[mental carbon]

Correct

[frail oriole]

to fix up the mess afterwards is going to be far more work than just waiting and doing the rebase later

[mental carbon]

Yep

take your time cpw

If you can't complete it

Then I will pick it up tomorrow again

[frail oriole]

deleteing hunks is fun 😄

[mental carbon]

It sure as hell is 😄

needs to create a new version of binpatcher

Anybody know a good name for the binary patcher?

[frail oriole]

?

[inland mesa]

Transmogrifier

[frail oriole]

keep seeing this error

dr-xr-xr-x 5 cpw cpw 4096 Sep 17 15:03 java

the directory it creates isn't writeable? @mental carbon

yeah, this is a weird one. it's creating read only dirs

which it then can't edit

[inland mesa]

gradle bug?

[frail oriole]

dunno

[inland mesa]

hmm no because this is generated by the setup task

so it's probably using java code

[frail oriole]

yeah

i suspect it's just a mkdir permission error

[inland mesa]

mkdir should use full perm flags by default though

[mental carbon]

No on purpose XD

I need to disable that code

Give me a second to push that

[inland mesa]

wut

[dull bison]

what was that purpose

[mental carbon]

clean and neoform should not be editable

I used the same setup task

For the forge project XD

Just forgot to disable it there

Pushed

[frail oriole]

cool

but you will get the same error when rerunning setup if you forget the forge qualifier

i was getting it for neoform and clean

[mental carbon]

Yeah I might need to disable that globally

Ahead of unpack

[inland mesa]

read-only files sounds fine but a read-only folder also can't be deleted :p

[mental carbon]

And then reset it again when the unpack completes

[mental carbon]

Most people in this room do know how to remove the write-lock

[inland mesa]

yeah I guess avoiding mistakes beats having to chmod

[mental carbon]

Yeah

And I will code the platform module to prevent it from erroring

So hopefully that works

[mental carbon]

Interesting I don't actually get that message

Aaaah

It is because I am running this on an NTFS FS in Ubuntu

[frail oriole]

you have to get it working successfully TWICE

or that

[mental carbon]

Okey pushed an update

That should take care of it

It marks the directories again as writeable

[frail oriole]

cool

how do i gen patches?

neogradle/runtime/platform/createPatches?

clean:compileJava shouldn't run

hmmm

[mental carbon]

Currenly unpackPatches

Should be the one that generates the zip

[frail oriole]

yup. i got it

[fallow sundial]

unpackPatches

[mental carbon]

Yes

Cause that is literally what it does

It unpacks the patches zip generated by diffpatch

[frail oriole]

pushed a bunch of AT cleanup

[dull bison]

why are you using NTFS under ubuntu?

[fallow sundial]

did you forget to commit the AT file?

[mental carbon]

No

I had to make a bunch of patch files

With ATs

To get it to properly work during the migration

Cpw is just very nice and undoing it for me

[frail oriole]

the AT file hadn't changed

it was applying fine. we had a bunch of patches that undone AT because we were getting compiling and patching working

that's just the way it works

right

what next

there may be further stuff i didn't catch with my regex

but given it's compiling, i don't think they're significant

[mental carbon]

The next thing would either be runs or the binpatcher

I need to spend some quality time with our binpatcher tool

For some reason it wants SRG ids to do its bidding

And I am not willing to give it those

So I need to figure out what the f it wants

[frail oriole]

hmmm

grrrr FFS

[mental carbon]

?

[frail oriole]

ran "createPatches" - it redid setup and wiped out the local changes instead. THANKS

[mental carbon]

?

In ngnext?

[frail oriole]

i just ran createPatches after fixing some more stuffs

in ngnext yes

[mental carbon]

I don't remember having a task named createPatches?

[frail oriole]

some more AT stuffs missed

[mental carbon]

Only generate and unpack

[frail oriole]

:forge:unpackPatches

[mental carbon]

That should not run setup

[frail oriole]

it does

[mental carbon]

it should not override your working directory in src/main

[frail oriole]

it did

[mental carbon]

Aahh Right

It needs the packed current state

And since I made that an output

It now triggers that

My mistake

Let me see if I can fix that

[frail oriole]

heh

thanks

lost an hour's work. joy

[mental carbon]

My mistake

So sorry 😄

[frail oriole]

no worries

interesting, it seems to not regenerate 😦

[mental carbon]

Yeah

Because I set it to depend on setup

So as soon as you run unpackPatches

It will trigger generatePatches

Which will trigger setup

[stray scroll]

as i understand the BinaryPatcher (Generator class), it uses a (T)SRG file only for remapping between obf and srg class names, and only when provided
you should be able to leave the option out

[mental carbon]

Which immediatly overrides it

[stray scroll]

or perhaps you might want to provide a SRG file between obf and mojmap class names

[mental carbon]

https://github.com/neoforged/Transmogrifier/blob/56dffe5338cf029be2eb0cd3752efd3b6fdfa0b4/src/main/java/net/minecraftforge/binarypatcher/ConsoleTool.java#L47C66-L47C66

It seems to be a required arg

[stray scroll]

i've not checked the link, but I guess you are referring to withRequiredArg(), which means the option itself has a required argument -- not that the option itself is required

[mental carbon]

Ah okey.....

[stray scroll]

the option itself being required is configured by a required() call

[lean warren]

JOMLJoptSimple is totally not confusing

[stray scroll]

JOML, you say?

[mental carbon]

I already used gigas suggestion as you can see above

It is now called transmogrifier

@frail oriole Pushed a change

This unhooks the dependency

[lean warren]

You know what I mean 😄

[mental carbon]

Ah okey

That should make live a lot easier

[frail oriole]

hmmm

lets see

it's running a decompile

and patched

😦

[mental carbon]

Can you send me your log

If you still have it

It should not

Since I did not attach them

[frail oriole]

nevermind

[mental carbon]

?

[frail oriole]

it worked

[mental carbon]

Yeah

it will run a decompile

Because the toolchain changed

But it should not have run the setup tasks

Just the steps to get a clean neoform jar

And then used the existing files on disk

As the other input

Without running the actuall setup task

[frail oriole]

there you go

i think that's all the AT mistakes

[mental carbon]

🫂

That makes my live so much easier

[frail oriole]

i managed to find the work in local history

ok brb

[mental carbon]

Does anybody know if there is an online LZMA viwer somewhere?

[fallow sundial]

finding lzma docs is a horrible experience, I doubt that's going to go better

[stray scroll]

is that speaking from experience, maty

[fallow sundial]

yes

hmm, that might've been xdelta actually

[mental carbon]

I can just unzip our binary patches.....

The files themselves are xdelta

But I don't care about the contents

Just what the file structure of our binpatches is

I found something funny

We distribute three variations of the binary patches

One for userdev (joined), two for production (client and server), as far as I can tell, they are binary identical...........

Actually nevermind

They are different

But when the binary patch of the server is applied to the server

It will have all client classes

With all methods

As far as I can tell

Client patches on the left

Server patches on the right

No way to know what is in them
XDelta is no help there

But the relevant pack200 and lzma files contain the same file count

Which is logical

Since they are generated from the same joined patched results

Their clean is just different

client is generated from the vanilla clean mapped jar

while server is generated from the vanilla server mapped jar

But they are both generated against the mapped forge dirty jar.....

Which is hella interesting

I mean it is pretty difficult to not do this either

It would require treeshaking the joined patched jar again

Before comparing it with the clean jar

but @frail oriole If you have any insights here, they are welcome

[lean warren]

That would explain why RuntimeDistCleaner prints "attempted to load class X/Y for invalid dist Z" on a production dedicated server instead of the JVM throwing a ClassNotFoundException (which would be the expected result in production)

[mental carbon]

Yep

Cause all classes are actually available

As long as they show up in the joined compiled output of forge (which they all do, cause it is generated from the joined mapped neoform/mcpconfig output)

Does anybody know if the classes contained in the server jar, which are also in the client jar, so the common classes, are binary equivalent?

[lean warren]

I tried to find out but the classes are not in the server JAR. The sizes are as such also wildly different

[prime quest]

what the heck is a Transmogrifier

[inland mesa]

"transmogrify" means to transform something by magical means

Orion asked for a name to give a binpatcher fork

and I proposed that

I assumed there would be other proposals lol

[rich void]

I suggested TwoTails

( more of a fox-related name than technical)

[tawny iron]

or..

we could name it something that actually means something useful

rather than everything being named as a joke that means nothing to anyone not already involved

[quiet talon]

Yeah, already kinda tired of the porting repo being named "Kits"

[tawny iron]

just kinda feels like everything is being treated as a joke

which is a bit disheartening cus that's terrible for longevity

[prime quest]

Everything being taken way too seriously is also terrible for longevity tbh

If having fun with it is disparaged and annoys people it very quickly turns into an annoyance itself

It's a hobby project. Let us have our fun :(

From my perspective, kits is a better name than "neoforge-private" because it doesn't induce that "omfg there's a private closed source version of Forge" kneejerk reaction that we used to get nearly constantly

And honestly, nearly everything that isn't the same is going to have the same complaints as kits, so this is the best we're gonna have

binpatcher, on the other hand, is already a fork of a fork

So changing it's name wasn't necessary :P

[mental carbon]

Good morning

[mossy bobcat]

Hey, transmogrifier is memorable and somewhat descriptive, so I'm not going to complain too much

[rich void]

well yeah that's what they went for. also, it is cooler than my idea xD

[dull bison]

if nobody else is doing it i'll look at runs after work (~7h)

[frail oriole]

Morning

Sorry I had to drop after yesterday. Got tired and ended up having an epic nap

[mental carbon]

That is fine

I am mostly interested in your answer with respects to the binary patches

[frail oriole]

I think we're in an ok place

What's the question?

[mental carbon]

Read from here down: #1136320550168436798 message

<@&1067092163520909374> RC2 Just dropped

[frail oriole]

I don't understand the question

[mental carbon]

Basically we create 3 different binary patches

[frail oriole]

Yeah, the patches patch both sides to a singular view

[mental carbon]

One for userdev from the joined clean and joined forge jar

[frail oriole]

Dunno about userdev

[stray scroll]

oh lawd it comin'

[mental carbon]

So it is intended that the binary patch for the server recreates all client classes?

[frail oriole]

Yes

Because what else are we going to do

[mental carbon]

Okey good to know.....

[frail oriole]

Basically we diff from a merged statr

We'd need to somehow demerge the built state

[mental carbon]

Yeah but we don't actually need to, anymore..... That was were I was going with my question

[frail oriole]

Fuck knows how we would do that

[mental carbon]

Would it be an idea to create a common/client/server architecture

[frail oriole]

Why not?

[mental carbon]

It would still create a universal setup in the end

[frail oriole]

Oh god please don't

[mental carbon]

Okey

Just wanted to get the feedback

[frail oriole]

That just sounds unnecessarily complicated

It's already complex enough

I don't know how you'd track all the various artifacts through from beginning to end either

I think you believe it's simple but it's not

So yeah I would just generate the binpatches from merged statr

Not sure why userdev requires another binpatch but whatever

[mental carbon]

Okey good to know

Makes my life easier

The installer much bigger

But my life easier

[frail oriole]

The installer should be the size it has always been

[mossy bobcat]

(accidentally dumped something in the wrong channel)

[dull bison]

if someone is doing something in the back channel you can pull me over if you want to

[mental carbon]

@fallow sundial or anybody else proficient in groovy can you tell me why this line gives me a method not found exception: https://gist.github.com/marchermans/cc821b77a1161fa7b8551bdb67bccd95#file-execute-groovy-L83

My IDE says the class is fine

And properly navigates me through it

But the Groovy compiler in gradle falls on its face

[mossy bobcat]

Runtime failure or compile time failure?

[mental carbon]

Compile time

Due to @CompileStatic

[mossy bobcat]

And what's DefaultMethods do?

(also this is groovy 3 right?)

[mental carbon]

Yeah

Okey I figured it out