FML Clean-up | The NeoForged Project | Page 3

glad cobalt Apr 14, 2024, 11:37 AM

#

Or some code that generates it

#

Unsure

haughty copper Apr 14, 2024, 11:37 AM

#

Yeah unsure myself. Patch might actually work 🤔 We could also have the installer generate it explicitly I suppose

#

Neoform adding it is probably a bad idea since it's NeoForge specific

#

Although the credits that are currently in the generated file refer directly to MCP

#

To which NeoForm would be the successor

glad cobalt Apr 14, 2024, 11:38 AM

#

I would have NeoGradle generate it as a Platform task

#

Probably

#

The question becomes neodev runs

#

How do you get it there

#

And how do you get the right MC version

haughty copper Apr 14, 2024, 11:38 AM

#

This is what we currently generate in code:

final var conf = Config.inMemory();
conf.set("modLoader", "minecraft");
conf.set("loaderVersion", "1");
conf.set("license", "Mojang Studios, All Rights Reserved");
final var mods = Config.inMemory();
mods.set("modId", "minecraft");
mods.set("version", FMLLoader.versionInfo().mcVersion());
mods.set("displayName", "Minecraft");
mods.set("logoFile", "mcplogo.png");
mods.set("credits", "Mojang, deobfuscated by MCP");
mods.set("authors", "MCP: Searge,ProfMobius,IngisKahn,Fesh0r,ZeuX,R4wk,LexManos,Bspkrs");
mods.set("description", "Minecraft, decompiled and deobfuscated with MCP technology");
conf.set("mods", List.of(mods));

#

I mean NeoForm already injects that ancient MCP entrypoint. It could also just inject this?

#

I always wondered about this thing. Does this even do anything? https://github.com/neoforged/NeoForm/blob/1.20.5-dev/versions/pre/1.20/1.20.5-pre1/inject/mcp/client/Start.java

glad cobalt Apr 14, 2024, 11:40 AM

#

It is normally needed for neoform runs only

#

But since NG7 we don't have those anymore

haughty copper Apr 14, 2024, 11:41 AM

#

Yeah in NG7 we could just start the original entrypoint

#

We also don't ship that logo anymore: mcplogo.png

#

No wait. We do 😄

#

In NeoForge itself

glad cobalt Apr 14, 2024, 11:42 AM

#

The MCP Metadata can be yeeted entirely 😄

#

We don't have MCP

haughty copper Apr 14, 2024, 11:42 AM

#

I know, we ship the logo still in NF Lol

#

#

We should just use this asset as the logo 😄

errant yew Apr 14, 2024, 11:59 AM

#

kekw

#

the end of an era, indeed

haughty copper Apr 14, 2024, 1:00 PM

#

Your thoughts: NeoforgeXyz or NeoForgeXyz in class-names?

#

Judging from NFs own code, It'd be NeoForge, right?

errant yew Apr 14, 2024, 1:09 PM

#

ye, NeoForge

jovial vault Apr 14, 2024, 1:11 PM

#

Nf

haughty copper Apr 14, 2024, 1:12 PM

#

We don't really have much of a use for the Common intermediary classes without FmlOnly:

#

For production use at least

#

@chrome mortar
Keep in 1.20.5?
// TODO - 1.21: remove the fallback to the mandatory field

chrome mortar Apr 14, 2024, 1:14 PM

#

remove

#

its not feasible to keep it since locators and providers are different and you can't know if it's from modclasses

haughty copper Apr 14, 2024, 1:15 PM

#

Yeah heh, that's why I stumbled upon this

chrome mortar Apr 14, 2024, 1:15 PM

#

and besides people will have to change the mods.toml

haughty copper Apr 14, 2024, 1:16 PM

#

I am triyng to split up the responsibilities of MinecraftLocator

#

The alternative would be to check if the primary path of the mod-file is a directory

restive stag Apr 14, 2024, 1:22 PM

#

Oh right, if you're changing locators and launch handlers I have some cursed code which hooks into neogradle for cross-project stuff which would be nice to include

haughty copper Apr 14, 2024, 1:22 PM

#

...?

#

You'll need to be more specific

#

A key thing I am currently trying is to "invert" control and have launch handlers only return additional locators/providers

restive stag Apr 14, 2024, 1:22 PM

#

https://github.com/FiniteReality/ng7-locator-demo/ was the version I showed to orion, who said they liked the idea in principle

haughty copper Apr 14, 2024, 1:23 PM

#

instead of the MinecraftLocator reaching back into the launch handler

#

So the production launch handlers will return a provider that knows how to get a IModFile for minecraft in a production environment

#

Or rather, one for server, one for client (since they differ)

restive stag Apr 14, 2024, 1:24 PM

#

yeah, makes sense; I just wanted to make sure that it'd work well with mod sources being passed via different means in userdev

#

since IIRC that's also done by the minecraftlocator stuff

haughty copper Apr 14, 2024, 1:25 PM

#

Maty already had split locators into two distinct concepts:

Locators -> ModFileCandidateLocator: Only responsible for finding potential (they return JarContents)
ModFileReader -> Turns JarContents into IModFile
ModFileProvider -> Creates IModFiles out of "thin" air (meaning they dont postprocess the result of a locator)

restive stag Apr 14, 2024, 1:25 PM

#

Awesome

haughty copper Apr 14, 2024, 1:25 PM

#

Yes but I am not at the point of MOD_CLASSES yet, but I want to split that off from the responsibility of whatever is providing the minecraft sources

#

The annoying part is that our platform provides the MC classes via MOD_CLASSES too

#

but that's actually not great

#

in NF dev

restive stag Apr 14, 2024, 1:26 PM

#

Sorry for getting in your way then 😅

haughty copper Apr 14, 2024, 1:26 PM

#

I'd probably favor detecting the MC classes via finding one of the MC classes in system classpath instead

#

But. Not there yet

restive stag Apr 14, 2024, 1:27 PM

#

I'll keep an eye out for the changes though so I can update my gradle plugin and then maybe get it upstreamed into NG

haughty copper Apr 14, 2024, 1:27 PM

#

Well what exactly is it doing?

#

I could look at the source, but can you summarize? 😛

restive stag Apr 14, 2024, 1:28 PM

#

It's passing mod classes via a gradle artifact, so an outgoing configuration variant

haughty copper Apr 14, 2024, 1:28 PM

#

what is the interface towards the actual mc entrypoint

restive stag Apr 14, 2024, 1:29 PM

#

So instead of using MOD_CLASSES it instead passes the classes via, currently, a command line parameter

chrome mortar Apr 14, 2024, 1:29 PM

#

so same thing

restive stag Apr 14, 2024, 1:30 PM

#

err, a system property but yeah, same thing

haughty copper Apr 14, 2024, 1:30 PM

#

You could potentially acually use an arg-file and have a task write that

restive stag Apr 14, 2024, 1:30 PM

#

yeah

haughty copper Apr 14, 2024, 1:30 PM

#

If this is for dev-only

restive stag Apr 14, 2024, 1:30 PM

#

that's pretty much how it works

haughty copper Apr 14, 2024, 1:30 PM

#

That way you would get away with IDE sync not having to resolve this

restive stag Apr 14, 2024, 1:31 PM

#

I can actually get away with it anyway, since there were things about the Gradle API I weren't familiar with when I first implemented the plugin

haughty copper Apr 14, 2024, 1:32 PM

#

We do some real wild shit in our launch handlers, jesus

restive stag Apr 14, 2024, 1:32 PM

#

Yeah...

#

~~while you're at it could you get rid of the log4j config stuff 😛 ~~

haughty copper Apr 14, 2024, 1:33 PM

#

client-1.20.4-20231207.154220-srg.jar

#

The srg classifier was just a renamed client? I Forget

#

It doesn't have the MCP start file so I'd assume it's completely unpatched

#

neoforge-20.4.214-client.jar is then just the jar with actually patched Minecraft client classes

haughty copper Apr 14, 2024, 5:05 PM

#

Alright I can run this from a unit test, that makes things a lot easier

    @Test
    void testDiscoverMods() throws Exception {
        environment.computePropertyIfAbsent(IEnvironment.Keys.LAUNCHTARGET.get(), ignored -> "forgeclient");

        FMLPaths.loadAbsolutePaths(gameDir);

        FMLLoader.onInitialLoad(environment, Set.of());
        FMLPaths.setup(environment);
        FMLConfig.load();
        FMLLoader.setupLaunchHandler(environment, versionInfo);
        FMLEnvironment.setupInteropEnvironment(environment);
        Environment.build(environment);

        var launchContext = new TestLaunchContext(environment);
        FMLLoader.beginModScan(launchContext);
    }

#

Well. Once. java.lang.RuntimeException: Environment is a singleton
Our code is really really not testable

jovial vault Apr 14, 2024, 5:11 PM

#

reflect that field to null first?

#

there's a test setup step right? or a "before each test" thing?

haughty copper Apr 14, 2024, 5:16 PM

#

Yeah sure, I am thinking about just lifting that restriction 😄

#

This is FMLs environment, so ... I can just change the build method

glacial crag Apr 14, 2024, 6:52 PM

#

Module org.apache.httpcomponents.httpclient reads more than one module named cpw.mods.securejarhandler
lovely 😄

#

why does this keep happening

#

I suppose that it gets loaded from the LCP for some reason

#

ah yes of course

#

it's the fucking ignoreList

#

-DignoreList=securejarhandler-2.1.24.jar,asm-9.5.jar,asm-commons-9.5.jar,asm-tree-9.5.jar,asm-util-9.5.jar,asm-analysis-9.5.jar,bootstraplauncher-1.1.2.jar,JarJarFileSystems-0.4.0.jar,client-extra,neoforge-

#

if there is any other version of SJH than 2.1.24 on the LCP then it gets added to the MC-BOOTSTRAP layer

chrome mortar Apr 14, 2024, 7:01 PM

#

can we somehow get rid of the ignore list

glacial crag Apr 14, 2024, 7:02 PM

#

that's part of the longer term goal

haughty copper Apr 14, 2024, 7:02 PM

#

In theory if the loading was more integrated

#

but unlikely for 1.20.5

#

one thing that might be done is have it go by modulename

#

I think it reads the jar descriptors anyway

#

Oh and wait, it can check that the module is already loaded

haughty copper Apr 14, 2024, 7:34 PM

#

Oh god, Spotless can't handle J21 code 😄

glacial crag Apr 14, 2024, 7:36 PM

#

harold

#

even if you update it?

haughty copper Apr 14, 2024, 7:36 PM

#

Yup

#

Chokes on this:

return switch (providerResult) {
    case null -> Optional.empty();
    case LoadResult.Success<IModFile>(var modFile) -> Optional.of(modFile);
    // Rethrow provider errors immediately, to be handled below
    case LoadResult.Error<IModFile>(var error) -> throw new EarlyLoadingException("JIJ ERROR", null, List.of(error));
};

glacial crag Apr 14, 2024, 7:38 PM

#

that's sad cause it's the perfect use case 😅

#

maybe we have to update spotless?

haughty copper Apr 14, 2024, 7:39 PM

#

I have no idea how

glacial crag Apr 14, 2024, 7:39 PM

#

it's applied by GradleUtils

haughty copper Apr 14, 2024, 7:39 PM

#

This seems to be in a transitive dependency:

Caused by: com.google.googlejavaformat.java.FormatterException: 76:45: error: ')' expected
    at com.google.googlejavaformat.java.FormatterException.fromJavacDiagnostics(FormatterException.java:51)

glacial crag Apr 14, 2024, 7:39 PM

#

maybe we can manually apply a higher version?

haughty copper Apr 14, 2024, 7:40 PM

#

Well, the FML project also depends directly on the spotless gradle plugin, and bumping that didn't help

glacial crag Apr 14, 2024, 7:40 PM

#

hmmm why does it use google-java-format; imports?

haughty copper Apr 14, 2024, 7:40 PM

#

Ah, yes RemoveUnusedImports

glacial crag Apr 14, 2024, 7:40 PM

#

https://github.com/google/google-java-format/issues/865

GitHub

Google Java Format fails on files that use record patterns in switc...

Using a record pattern in a switch (Java 19 preview) e.g.: int c = switch (r) { case Record(var a) -> a; case AnotherRecord(var b) -> b; default -> 0; } causes Google Java Format to fail w...

#

pffff

haughty copper Apr 14, 2024, 7:41 PM

#

Thanks Google!

glacial crag Apr 14, 2024, 7:42 PM

#

I suppose this also blocks us from using record patterns in switch in neoforge until we find a workaround

haughty copper Apr 14, 2024, 7:42 PM

#

I am pushing my work in progress state. Lots of TODOs left

thin mica Apr 14, 2024, 7:43 PM

#

glacial crag maybe we have to update spotless?

no but we can always yeet it

glacial crag Apr 14, 2024, 7:44 PM

#

that's a possible workaround but it's annoying 😛

haughty copper Apr 14, 2024, 7:51 PM

#

Talking testcase, I'd run through FMLLoader init, beginScan/completeScan and then check essentially three things:

The layer contents returned by beginScan and completeScan, as well as the LoadingModList errors+warnings+mods

#

From my understanding, currently we do not cancel loading if a locator fails, it just gets recorded in the loadingmodlist and shown later, right?

glad cobalt Apr 14, 2024, 7:57 PM

#

Yes

#

We need to keep loading

#

As we need to discover MC and Neo

chrome mortar Apr 14, 2024, 8:02 PM

#

could we disable the remove imports rule

#

i don't want a code formatter to prevent us from using new language features, that's silly australian_tater

haughty copper Apr 14, 2024, 8:05 PM

#

Especially with no real horizon on google fixing it

#

We'll have to add error-display capabilities to early loading later

glad cobalt Apr 14, 2024, 8:05 PM

#

haughty copper We'll have to add error-display capabilities to early loading later

No...

haughty copper Apr 14, 2024, 8:05 PM

#

Right now there's no real way to handle the error condition that arises when minecraft or neoforge is missing

#

Yeah we have to, Orion

glad cobalt Apr 14, 2024, 8:05 PM

#

Ahh

#

That error

#

Yeah that is fine

haughty copper Apr 14, 2024, 8:05 PM

#

There's no way to handle certain errors

glad cobalt Apr 14, 2024, 8:06 PM

#

Yeah

#

But they are never actually missing

haughty copper Apr 14, 2024, 8:06 PM

#

And we can't fall back to AWT/Swing since early display already hijacked the main thread for GL 😄

#

Well they can be if your install is corrupted and I'd rather show a nice error message

#

Currently we just crash the process with an exception that swallows the original error

#

Well I'll just try to barf out the currently queued errors & warnings from early loading and barf out the exception, not much else we can currently do

#

(barf out -> to the log)

glacial crag Apr 14, 2024, 8:22 PM

#

chrome mortar could we disable the remove imports rule

probably yeah - can we make it opt-out in GU?

haughty copper Apr 14, 2024, 9:14 PM

#

I have to wrap my head around system mods again 😅

glacial crag Apr 14, 2024, 9:16 PM

#

it's just a little hack to load mc and neoforge even if there are loading errors

haughty copper Apr 14, 2024, 9:24 PM

#

"just a little hack" ❤️

mild storm Apr 17, 2024, 12:29 AM

#

RE: to multiple comments on https://github.com/neoforged/NeoForge/pull/774/files#diff-f6dcf88ce2e4c294e2bb0002de41bb587303e6dd66b75b4b2d76ee1e4af2e3c6

Big +1 to moving stuff to static methods on ModLoader instead of having stuff like this

glacial crag Apr 17, 2024, 9:11 AM

#

It doesn't really matter

haughty copper Apr 17, 2024, 9:25 AM

#

For unit testing purposes, it might be beneficial to have an instance hidden behind get() that can be reset

glacial crag Apr 17, 2024, 11:43 AM

#

Eh idk

glacial crag Apr 17, 2024, 7:40 PM

#

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

GitHub

Make ModLoader fully static, remove duplicate class by Technici4n ·...

#

this makes all the methods in ModLoader static

#

we should probably separate out the actual loading methods and the methods to dispatch mod bus events?

#

not this time though I think - I want this refactor finished at this point

haughty copper Apr 17, 2024, 7:48 PM

#

check my comment on GH

#

those static fields introduce potential classloading footguns

glacial crag Apr 17, 2024, 7:51 PM

#

I saw it and fixed it

glacial crag Apr 17, 2024, 7:52 PM

#

haughty copper those static fields introduce potential classloading footguns

the whole thing was already static before - call any of the methods too early and you'd have the same problem

#

the footguns have always been there 😛

haughty copper Apr 17, 2024, 7:53 PM

#

glacial crag the whole thing was already static before - call any of the methods too early an...

It still requires a call to the method. Now it would just require loading the class 😛

glacial crag Apr 17, 2024, 7:58 PM

#

and calling one of its methods

#

classloading is not enough to run the static initializer 😉

haughty copper Apr 17, 2024, 7:59 PM

#

Well, any, though 😛 Eh IDK, it's just not great trying to trace bugs like these later

#

Hmmmm, can I even test how a SecureJar was built?

#

I.e. for testing FML launch, I should check that it built the minecraft jar from the right components with the right filter

#

I suppose I could try querying files in it to check that it doesnt return neoforge classes

glacial crag Apr 17, 2024, 8:10 PM

#

yeah I guess

buoyant shoal Apr 17, 2024, 8:16 PM

#

Could Bindings also be made fully static? It has an instance field but it's private and all public methods are static anyway.

glacial crag Apr 17, 2024, 8:17 PM

#

that needs more cleanup anyway

#

and it's not really relevant to complexity - it's just a very minor cleanup

buoyant shoal Apr 17, 2024, 8:19 PM

#

Alright

haughty copper Apr 17, 2024, 9:43 PM

#

Hm I should also make sure I only set librariesDirectory in tests for launch targets that actually would have it set, right? 😄

#

As far as I can tell that would only be production client & production server

#

I feel like I am close to recreating DataResult light concern

chrome mortar Apr 17, 2024, 9:55 PM

#

it's bedtime grandpa

haughty copper Apr 17, 2024, 9:56 PM

#

It's the goddamn exception handling I need to do for JarContents.of

steel egret Apr 18, 2024, 11:27 AM

#

Hey everyone - I've been away from the scene for quite a while (last I worked on minecraft, js coremods were not a thing). Back then, I benchmarked loading and came to the conclusion that most time during loading was spend reading the bytecode into asm trees and back. I think I even proposed having coremods just work on trees directly, but since the JS stuff was already underway, this wasn't really feasible.

I'm curious what the state of coremods is currently, and what the bottlenecks of loading are

eternal wren Apr 18, 2024, 11:27 AM

#

Feel free to benchmark again

#

But as far as I'm aware, ideas to avoid full reading are already being considered

#

If that's a hot point anyway

steel egret Apr 18, 2024, 11:28 AM

#

Yeah, I came here from the new year's blog post, but it links a thread that I don't believe exists anymore

#

yeah, I managed to found the issue I made in 2018 but I didn't include the benchmarks there

#

oh well

steel egret Apr 18, 2024, 12:05 PM

#

Ok, just found this pr https://github.com/neoforged/FancyModLoader/pull/79/files and it looks pretty good! 🙂

GitHub

Add Java Core Mods by shartte · Pull Request #79 · neoforged/FancyM...

Mods must ship a separate Jar-file (for example via jar-in-jar) and mark it as FMLModType: LIBRARY to make it load above the GAME layer. They then must provide an implementation of ICoreMod via the...

glacial crag Apr 18, 2024, 12:26 PM

#

A lot of the slowdown comes from initializing the coremod engine, hence the move to Java

#

Another big slowdown is parsing every single class with ASM - we also had plans to address that

steel egret Apr 18, 2024, 12:43 PM

#

the targetting methods in that pr are related to that no?

#

i have not looked at this code in 6 years, but it looks better than what I remember

glacial crag Apr 18, 2024, 2:05 PM

#

steel egret the targetting methods in that pr are related to that no?

Kind of

#

The problem is that we have other asm transforms that target every class

steel egret Apr 18, 2024, 2:13 PM

#

and those get passed the raw bytes?

glacial crag Apr 18, 2024, 3:52 PM

#

No but they cause the ClassNode to be parsed for every class

steel egret Apr 18, 2024, 5:12 PM

#

Ah, they get passed a node - when I did my local modification way back what I did was simply add another interface that takes a tree instead

#

if two of that interface came in a row in the transformers, you could skip converting out of the tree

glacial crag Apr 18, 2024, 5:14 PM

#

What tree??

steel egret Apr 18, 2024, 5:24 PM

#

Sorry - I confused the node hiearchy and reader

#

by tree I meant node

#

and I assumed they were passed classreader/classwriters

#

as in the visitor api

#

i really should just profile it again

#

I see the problem - not all classes need to be transformed, so you wouldn't need to create a tree for all of them

#

there is currently no api for specifying "I only care about x class" right?

haughty copper Apr 18, 2024, 5:30 PM

#

Well, targets

plain solar Apr 18, 2024, 5:38 PM

#

steel egret there is currently no api for specifying "I only care about x class" right?

there is, but most (all?) of the transformers in Neo all need to decide based on things like the presence of an annotation

#

and since we don't have an API for filtering a class based on the constant pool, the transformers currently have to say "yes" to every class

haughty copper Apr 18, 2024, 5:39 PM

#

yup, we'd need "target class with annotation X", "target method with annotation Y", and as embeddedt said, THEN we could pre-filter by constant pool

steel egret Apr 18, 2024, 5:40 PM

#

Ah, that constant pool idea is crazy neat

#

Ats operate on the visitor api right?

haughty copper Apr 18, 2024, 5:43 PM

#

ATs now operate on source

#

Well, *in addition

steel egret Apr 18, 2024, 5:43 PM

#

But when you load into retail

haughty copper Apr 18, 2024, 5:43 PM

#

Yes, ignore me .D

#

I just had matys recent PR in my head

steel egret Apr 18, 2024, 5:44 PM

#

Ye ye no worries

chrome mortar Apr 18, 2024, 5:45 PM

#

steel egret Ats operate on the visitor api right?

no, nodes

steel egret Apr 18, 2024, 5:46 PM

#

Then won't most classes need to be turned into a tree anyways?

#

Or am I assuming that most classes need ats when that is not case

plain solar Apr 18, 2024, 5:49 PM

#

steel egret Or am I assuming that most classes need ats when that is not case

most don't need ATs as far as I know

steel egret Apr 18, 2024, 5:50 PM

#

Got it

haughty copper Apr 18, 2024, 6:17 PM

#

Can we just close this? https://github.com/neoforged/FancyModLoader/pull/116

#

I do not see why they would need this. The supposedly hardcoded bits are in a class that's already loaded via serviceloader

chrome mortar Apr 18, 2024, 6:22 PM

#

yes

haughty copper Apr 18, 2024, 11:15 PM

#

Hm it would actually be nive to have a .neoforgeassetsroot to mark the folder/jar that contains our stuff

#

that'd allow us to get rid of MOD_CLASSES for neoforge dev at least 🤔

#

I could search for /neoforge_logo.png I guess lol

glacial crag Apr 18, 2024, 11:26 PM

#

haughty copper that'd allow us to get rid of MOD_CLASSES for neoforge dev at least 🤔

it's also used to run our tests, beware

haughty copper Apr 18, 2024, 11:27 PM

#

To add them into the minecraft mod jar?

glacial crag Apr 18, 2024, 11:28 PM

#

just to load them

haughty copper Apr 18, 2024, 11:29 PM

#

yeah okay I am strictly talking about auto-detecting paths to MC/NF themselves

glacial crag Apr 18, 2024, 11:29 PM

#

I have minecraft%%C:\Users\bruno\java\Kits\projects\neoforge\build\resources\main;minecraft%%C:\Users\bruno\java\Kits\projects\neoforge\build\classes\java\main;testframework%%C:\Users\bruno\java\Kits\testframework\build\resources\main;testframework%%C:\Users\bruno\java\Kits\testframework\build\classes\java\main;tests%%C:\Users\bruno\java\Kits\tests\build\resources\main;tests%%C:\Users\bruno\java\Kits\tests\build\classes\java\main on my machine 😛

haughty copper Apr 18, 2024, 11:30 PM

#

All of the insane filtering is done since the build output directory of projects/neoforge will contain classes from both MC and NF, right?

#

while build/resources will contain only the NF resources, and the MC client resources from client-extra.jar

glacial crag Apr 18, 2024, 11:31 PM

#

yeah

haughty copper Apr 19, 2024, 9:32 AM

#

Ooof.

#

I was trying to figure out why my neoforge module showed up as main

#

Then I remembered

#

We're currently creating a SecureJar based on the classes/resources in Neoforge dev

#

Then we return the root path of that as a locator location, and then it builds ANOTHER securejar based on that 😄

haughty copper Apr 19, 2024, 11:03 AM

#

Hrmpf, there is currently not a single class that's unique to the dedicated server?

tawdry light Apr 19, 2024, 11:08 AM

#

Been that way for a while

jovial vault Apr 19, 2024, 11:10 AM

#

yeah, aroudn 1.16 or so?

#

they stopped stripping dedicated server main from the client jar

#

OnlyIn(dedicated_server) hastn' been a thing since then

haughty copper Apr 19, 2024, 11:19 AM

#

Makes testing the FML startup slightly harder but oh well

#

I can still test for the absence of client classes

haughty copper Apr 19, 2024, 2:41 PM

#

This one test case covers ~30% of all lines in FML

#

Not a bad starting point 😅

glacial crag Apr 19, 2024, 3:36 PM

#

Nice ^^

haughty copper Apr 19, 2024, 7:02 PM

#

If a jar without a MANIFEST and without a neoforge.mods.toml is in mods/, we don't load it at all?

chrome mortar Apr 19, 2024, 7:03 PM

#

haughty copper If a jar without a MANIFEST and without a neoforge.mods.toml is in mods/, we don...

that's correct, we're meant to show a warning

#

it should go into unclaimed and then the other loader checks can propose an error

haughty copper Apr 19, 2024, 7:03 PM

#

okay that warning needs a test, currently we don't 😅

chrome mortar Apr 19, 2024, 7:04 PM

#

currently as in current FML?

haughty copper Apr 19, 2024, 7:04 PM

#

No, the locator branch

chrome mortar Apr 19, 2024, 7:04 PM

#

ah well i never tested so yes do test and fix

haughty copper Apr 19, 2024, 7:04 PM

#

I am writing unit tests for this stuff onw

chrome mortar Apr 19, 2024, 7:55 PM

#

looking at the PR, if a jij mod doesn't have a FMLModType manifest it seems like it will be ignored https://github.com/neoforged/FancyModLoader/pull/112/files#diff-bfd0b9684b6434f8854cc6aef63a70450237cdb08fa712b541ab9f7ba9367c9eR73

#

oh nvm i found NestedLibraryModProvider

#

that needs a bit of commenting about its existence in the jij code 😛

haughty copper Apr 19, 2024, 8:09 PM

#

Wait in which part of JIJ?

#

The FML side wrapper?

chrome mortar Apr 19, 2024, 8:09 PM

#

mhh

haughty copper Apr 19, 2024, 8:21 PM

#

@chrome mortar

#

So in case we do not identify a "jar problem", do we just load it as a library? 😅

#

or, if we do not, what do we do instead? like, what is the message we warn with

chrome mortar Apr 19, 2024, 8:22 PM

#

haughty copper So in case we do *not* identify a "jar problem", do we just load it as a library...

you don't do anything

#

should probably be a light warning

#

"i have no idea what to do with this jar"

#

not sure if it deserves a loading warn

steel egret Apr 19, 2024, 8:24 PM

#

If you do nothing with it, it should be a crash, if you add it as a lib, it should be a warning (opinion)

haughty copper Apr 19, 2024, 8:27 PM

#

If we add it as a lib, it should definitely not be a warning

#

If we don't do anything with it... hm

split wasp Apr 19, 2024, 8:33 PM

#

Are these root jars or JiJed stuff?

#

Root jars I assume?

haughty copper Apr 19, 2024, 8:50 PM

#

these are root

#

The nested library reader will take jars that have a parent mod-file and turn them into libraries

#

but if they do not have a parent mod-file it ignores them

#

I think the reasoning there was: if it's a plain jar that was jar-in-jar included in a mod, it likely has a reason to be there

#

while a loose file found in mods/ might just be an "accident" by the player

split wasp Apr 19, 2024, 8:55 PM

#

Yeah. Hmm. Do we have a reason not to just load it as a library? Especially if we've found no "issue" with it?

#

I'm not entirely sure here what makes more sense

haughty copper Apr 19, 2024, 8:56 PM

#

I'd personally just load it, but I also have no real-life examples of someone doing this

split wasp Apr 19, 2024, 8:58 PM

#

I can't think of scenarios where a mod with nothing would be in the root folder and you'd want to load it, so maybe error/warn loudly?

haughty copper Apr 19, 2024, 8:59 PM

#

true it could be a file from a modloader we just didn't account for

#

oooor it could just be someone throwing in log4j.jar for some reason 😄

split wasp Apr 19, 2024, 9:01 PM

#

haughty copper true it could be a file from a modloader we just didn't account for

In that case it'll have a mod type I'd assume, or something else to get it detected

haughty copper Apr 19, 2024, 9:03 PM

#

What?

#

If it's a modloader we didn't account for, it'll just not be a file we detect

#

Or what do you mean?

#

We try to detect a few cases:

#

@chrome mortar Why didn't we add our old mods.toml to this thing, btw?

#

Wait, we did

#

ignore me 😄

#

I just read OLDFORGE and stopped reading

split wasp Apr 19, 2024, 9:04 PM

#

haughty copper If it's a modloader we didn't account for, it'll just not be a file we detect

Oh, misread that

split wasp Apr 19, 2024, 9:04 PM

#

haughty copper oooor it could just be someone throwing in log4j.jar for some reason 😄

In which case yeah you'd probably want to error in either of those cases

haughty copper Apr 19, 2024, 9:05 PM

#

But wait, wasn't it META-INF/mods.toml?

split wasp Apr 19, 2024, 9:06 PM

#

Yep. quilt's quilt.mod.json file is missing too

chrome mortar Apr 19, 2024, 9:08 PM

#

haughty copper But wait, wasn't it META-INF/mods.toml?

oh yeah

#

remember to add translations to neo when bumping fml

haughty copper Apr 19, 2024, 9:08 PM

#

I have to do a pass on the localized error messages

#

I don't like how we handle those at all atm

#

(translation keys splattered across the codebase)

#

but anyway, so I have to change that to META-INF/mods.toml, don't i? Otherwise we'll silently ignore mods that forget to rename theirs mods.toml

chrome mortar Apr 19, 2024, 9:10 PM

#

shartte you have like 5 days most left

haughty copper Apr 19, 2024, 9:10 PM

#

I know my dude, the timing sucks 😄

#

but moving the translation keys to an enum should be doable

glacial crag Apr 19, 2024, 9:10 PM

#

good thing you can spend the whole weekend on this HAHA!

#

joking I hope

haughty copper Apr 19, 2024, 9:10 PM

#

Not really

#

That is my plan

#

😐

glacial crag Apr 19, 2024, 9:10 PM

#

😄

#

a good plan certainly

haughty copper Apr 19, 2024, 9:11 PM

#

Quick

#

Someone gimme a quilt mod

#

Is their quilt.mod.json in the file root

#

or under META-INF?

split wasp Apr 19, 2024, 9:11 PM

#

haughty copper Someone gimme a quilt mod

Root

#

https://www.curseforge.com/minecraft/mc-mods/excavated-variants/files/5166316

haughty copper Apr 19, 2024, 9:12 PM

#

Awesome thank you, will add that

haughty copper Apr 19, 2024, 9:30 PM

#

@chrome mortar why do we still have EarlyLoadingException vs. ModLoadingWarning/ModLoadingException?

glacial crag Apr 19, 2024, 9:39 PM

#

the way I understand it, ModLoadingException is a single exception whereas LoadingFailedException is an aggregate of them

#

no idea why there's other types 😛

haughty copper Apr 19, 2024, 9:40 PM

#

EarlyLoading is an aggregate

#

but ModLoadingWarning is just an aggregate too

#

Or rather, it is being aggregated, that way around

glacial crag Apr 19, 2024, 9:40 PM

#

my god

haughty copper Apr 19, 2024, 9:40 PM

#

What is more annoying is the different representations for basically the same thing

#

(i18n translation key, translation args, mod-file info)

glacial crag Apr 19, 2024, 9:41 PM

#

that can probably be cleaned up

haughty copper Apr 19, 2024, 9:41 PM

#

#

This message is not terribly easy to assert against so I gotta find an alternative for the paths: ["[fml.modloading.brokenfile[C:\Users\SEBAST~1\AppData\Local\Temp\gameDir16649063246816979246\mods\plainmod.jar]]"]

glacial crag Apr 19, 2024, 9:52 PM

#

the damned short paths

#

I can't unsee them now 😄

haughty copper Apr 19, 2024, 9:54 PM

#

the entire thing is just quite annoying to match against

eternal wren Apr 19, 2024, 10:03 PM

#

glacial crag the damned short paths

You don't like shortened paths?

haughty copper Apr 19, 2024, 10:07 PM

#

How about we try to center reporting around this?

public record ModLoadingIssue(
        Severity severity,
        String translationKey,
        List<Object> translationArgs,
        @Nullable
        Path affectedPath,
        @Nullable
        IModFile affectedModFile,
        @Nullable
        IModInfo affectedMod
) {
    public static ModLoadingIssue warning(String translationKey, Object... args) {
        return new ModLoadingIssue(Severity.WARNING, translationKey, List.of(args), null, null, null);
    }

    public ModLoadingIssue withAffectedPath(Path affectedPath) {
        return new ModLoadingIssue(severity, translationKey, translationArgs, affectedPath, null, null);
    }

    public ModLoadingIssue withAffectedModFile(IModFile affectedModFile) {
        var affectedPath = affectedModFile.getFilePath();
        return new ModLoadingIssue(severity, translationKey, translationArgs, affectedPath, affectedModFile, null);
    }

    public ModLoadingIssue withAffectedMod(IModInfo affectedMod) {
        var affectedModFile = affectedMod.getOwningFile().getFile();
        var affectedPath = affectedModFile.getFilePath();
        return new ModLoadingIssue(severity, translationKey, translationArgs, affectedPath, affectedModFile, affectedMod);
    }

    public enum Severity {
        WARNING,
        ERROR
    }
}

#

Dunno if we should opt to encapsulate the "context" in a typed map or similar to make it extensible

#

Or if we should abstract ModLoadingIssue, ModLoadingError extends ModLoadingIssue, etc. to require an error or a warning via the type system

chrome mortar Apr 19, 2024, 10:16 PM

#

haughty copper <@561254664750891009> why do we still have EarlyLoadingException vs. ModLoadingW...

feel free to change it, I didn't feel like it

haughty copper Apr 19, 2024, 10:16 PM

#

I am trying to figure something out for it right now. In tests I'd likely wanna check that a certain translation code + specific args are emitted as warnings hm

plain solar Apr 19, 2024, 10:16 PM

#

the early loading screen is still opened before locators run after this revamp, right

haughty copper Apr 19, 2024, 10:17 PM

#

No change there, yes

#

It's loaded in a very weird location

plain solar Apr 19, 2024, 10:17 PM

#

unfortunate but not really avoidable without introducing the exact problem it tried to solve

haughty copper Apr 19, 2024, 10:17 PM

#

in the hook that discovers additional plugins for ML I think

#

hm wdym embedded?

plain solar Apr 19, 2024, 10:18 PM

#

I really don't like my top-level jar having to be a SERVICE in order to access things like GraphicsBootstrapper

#

but a locator rewrite that allows locating JiJed services still wouldn't solve the problem because the locators aren't called upon that early

glacial crag Apr 19, 2024, 10:19 PM

#

JiJ'ing service jars just doesn't make sense IMO

#

at least not right now

#

it wouldn't work with SPL and other locators

plain solar Apr 19, 2024, 10:19 PM

#

yeah but like, can I even do compileOnly on a mod with that proposed jar layout (SERVICE + inner JiJed mod)

glacial crag Apr 19, 2024, 10:20 PM

#

no, it would likely need different maven artifacts

plain solar Apr 19, 2024, 10:20 PM

#

glacial crag + it wouldn't work with SPL and other locators

and this is the other problem, GraphicsBootstrapper is unusable if I'm being loaded through SPL

haughty copper Apr 19, 2024, 10:20 PM

#

I'll look at that once this stuff is done for 1.20.5

#

i dont think the startup flow makes a ton of sense

glacial crag Apr 19, 2024, 10:20 PM

#

plain solar and this is the other problem, `GraphicsBootstrapper` is unusable if I'm being l...

that is just unsolvable

haughty copper Apr 19, 2024, 10:20 PM

#

but we'll get to it

plain solar Apr 19, 2024, 10:21 PM

#

glacial crag that is just unsolvable

sadly

haughty copper Apr 19, 2024, 10:21 PM

#

What does that do exactly?

#

GraphicsBootstrapper in your contextß

plain solar Apr 19, 2024, 10:21 PM

#

lets me control OpenGL stuff before the first window is opened

haughty copper Apr 19, 2024, 10:22 PM

#

And it's not completely unsolvable, the issue is more that SPL will likely want to ship with earlydisply if we do neoforge-as-a-mod

#

if it doesn't it wouldn't have any progress indication

plain solar Apr 19, 2024, 10:25 PM

#

haughty copper And it's not completely unsolvable, the issue is more that SPL will likely want ...

why would SPL need to ship with earlydisplay? It's in FML not Neo

haughty copper Apr 19, 2024, 10:27 PM

#

We would not want to install FML into the launcher profile since it'd mean we cannot update it during that MC versions lifetime

#

I should clarify. FML with everything it includes now*

#

I suppose the argument could be made that earlydisplay doesn't change often

#

but I'd still be somewhat afraid 😄

plain solar Apr 19, 2024, 10:33 PM

#

haughty copper We would not want to install FML into the launcher profile since it'd mean we ca...

Why? People can update Fabric Loader (even if it's often not required).

haughty copper Apr 19, 2024, 10:34 PM

#

I think the idea is to try and never get into that situation of having to explain that

#

Also fabric loader doesn't have anything like earlydisplay, I think

glacial crag Apr 19, 2024, 10:37 PM

#

haughty copper And it's not completely unsolvable, the issue is more that SPL will likely want ...

well no - the problem is that SPL wants to show its progress in a window

haughty copper Apr 19, 2024, 10:37 PM

#

Hm?

#

Well yes and if the platform doesnt have it, it could ship earlydisplay itself 😛

glacial crag Apr 19, 2024, 10:37 PM

#

obviously this means that you can't run SPL-downloaded/managed code before the window is opened

haughty copper Apr 19, 2024, 10:37 PM

#

as in JIJ ship

glacial crag Apr 19, 2024, 10:38 PM

#

well that doesn't matter

#

the problem is that embeddium needs to tweak some things before the window is opened

haughty copper Apr 19, 2024, 10:38 PM

#

Well it doesnt get embedded what he wants, yes 😄

#

I know i know

glacial crag Apr 19, 2024, 10:39 PM

#

ok yes we agree on that

haughty copper Apr 19, 2024, 10:39 PM

#

The biggest problem was mac not allowing to re-create the gl context or something?

glacial crag Apr 19, 2024, 10:39 PM

#

potentially yes

#

also, "everything" (every render call?) must be on the main thread

glad cobalt Apr 19, 2024, 10:39 PM

#

Personal opinion: installing fml with the profile is fine.

#

We rarely update it

#

Especially not during an mc cycle

glacial crag Apr 19, 2024, 10:40 PM

#

well we don't want to be in the situation where we need to tell users to update NeoForge and FML

haughty copper Apr 19, 2024, 10:40 PM

#

I think we can just do better

#

FML is quite big and tied quite closely to NF

glad cobalt Apr 19, 2024, 10:40 PM

#

Why? Is it really worth all that extra Engineering and complexity

glacial crag Apr 19, 2024, 10:40 PM

#

haughty copper FML is quite big and tied quite closely to NF

that too

glad cobalt Apr 19, 2024, 10:40 PM

#

To have discovery in two places

haughty copper Apr 19, 2024, 10:40 PM

#

It's not that much engineering, really

#

We already have two ways of discovery

#

with the early-transformer discovery FML exposes to mL

#

I think it's too early for specifics anyway

plain solar Apr 19, 2024, 10:45 PM

#

I have an alternate solution already, it's just not ideal since it requires the user to reconfigure the early window provider manually in each new instance

#

but it's better than nothing

haughty copper Apr 19, 2024, 10:45 PM

#

What are you reconfiguring?

plain solar Apr 19, 2024, 10:45 PM

#

I disable the early window and then Embeddium starts it manually later

#

this approach works through SPL too

#

but obviously introduces the flaw of the screen showing up later than desired if SPL is really slow

haughty copper Apr 19, 2024, 10:47 PM

#

Yeah that is the same as SPL not having an early display and just not showing progress

#

I mean ideally you could show an OS window and then replace it later with GL

#

but as I understand it, we cannot show any OS-level windows in the same thread we later show the GL window in?

plain solar Apr 19, 2024, 10:47 PM

#

we can't show AWT/Swing windows, and I don't think Java provides an alternative?

haughty copper Apr 19, 2024, 10:47 PM

#

Or am I mixing that up with AWT trying to process window messages in another thread than thread 1?

#

Well if you can open&destroy a window in the same thread even on mac

#

you could just destroy/reopen the early display window

restive stag Apr 19, 2024, 10:49 PM

#

You should be able to create a new window but I don't know if the GL context will work properly

haughty copper Apr 19, 2024, 10:50 PM

#

Yeah IDK, we might test it at some point, it would not be the greatest UX I suppose but might work

#

Uuuuh. we error if a mod has no license set?

#

jovial vault Apr 19, 2024, 10:51 PM

#

yes, since the license line was introduced

haughty copper Apr 19, 2024, 10:51 PM

#

Huh

#

TIL

jovial vault Apr 19, 2024, 10:51 PM

#

it has never been valid to not include it

haughty copper Apr 19, 2024, 10:51 PM

#

But why don't we do that in the validator

#

This is in ModLoader

jovial vault Apr 19, 2024, 10:52 PM

#

I don't know the code for this

haughty copper Apr 19, 2024, 10:52 PM

#

Yeah ok, something to look at

jovial vault Apr 19, 2024, 10:52 PM

#

I just know that cpw really wanted it to be a required value

chrome mortar Apr 19, 2024, 10:52 PM

#

that piece of code is stupid

haughty copper Apr 19, 2024, 10:52 PM

#

It should be in the validator, right?

chrome mortar Apr 19, 2024, 10:52 PM

#

it should be where everything else is

glacial crag Apr 19, 2024, 10:52 PM

#

in general, our mods.toml validation is all over the place

haughty copper Apr 19, 2024, 10:54 PM

#

I suppose you validate this later since you want it to cover modsfiles that potentially use a different metadata format than mods toml

#

but ModValidator#validateFiles seems to be the right place for it

#

Hm no, not true, it doesnt have IModFileInfo at that point

#

Oh no, ofcourse it does. Nvm

#

Any objections to just always generating this error even if the file has other errors too?

split wasp Apr 19, 2024, 10:59 PM

#

plain solar yeah but like, can I even do `compileOnly` on a mod with that proposed jar layou...

If the mod declares an apiElements dependency on the inner one, then yes

plain solar Apr 19, 2024, 11:34 PM

#

haughty copper you could just destroy/reopen the early display window

not usable for my use case because as far as I know the change has to be done before the first GL window is opened

#

tbh it's very unlikely anyone else needs such specialized timing

haughty copper Apr 19, 2024, 11:37 PM

#

plain solar not usable for my use case because as far as I know the change has to be done be...

well i was kinda aiming at what change you are talking about 😄

glacial crag Apr 19, 2024, 11:37 PM

#

btw do we want to do https://github.com/neoforged/FancyModLoader/issues/101 as well?

GitHub

Move `EventBusSubscriber` into outer class · Issue #101 · neoforged...

EventBusSubscriber is currently an inner class of Mod for no obvious reason. This creates a double inner class and makes EventBusSubscriber less convenient to use. Additionally, according to Orion,...

#

it's very easy to write

plain solar Apr 19, 2024, 11:37 PM

#

the hacks sodium applies to shut off nvidia's buggy driver optimizations

glacial crag Apr 19, 2024, 11:37 PM

#

just a little bit breaking 😄

haughty copper Apr 19, 2024, 11:37 PM

#

You should be able to tear it down and reinit, but with macos i take nothing for granted anymore

haughty copper Apr 19, 2024, 11:38 PM

#

glacial crag btw do we want to do https://github.com/neoforged/FancyModLoader/issues/101 as w...

Thought about it, and yes. But make it a separate P R:D

glacial crag Apr 19, 2024, 11:38 PM

#

of course

haughty copper Apr 19, 2024, 11:38 PM

#

plain solar the hacks sodium applies to shut off nvidia's buggy driver optimizations

Do NVidia drivers even apply on MacOS?

plain solar Apr 19, 2024, 11:38 PM

#

no

glacial crag Apr 19, 2024, 11:38 PM

#

problem solved 😛

haughty copper Apr 19, 2024, 11:38 PM

#

but yeah it's likely per-proecss and that complicates things

plain solar Apr 19, 2024, 11:38 PM

#

the hacks are way too egregious for Neo to include directly

#

even as a feature flag

haughty copper Apr 19, 2024, 11:39 PM

#

It just wont work with SPL then

#

SPL is already a big headache for what i had in mind to protoype

#

due to early display 😄

glacial crag Apr 19, 2024, 11:40 PM

#

did you know? the "Y" in SPL stands for safetY

#

😛

#

downloading a neoforge jar through SPL would be quite cursed

haughty copper Apr 19, 2024, 11:40 PM

#

well yes, but it is convenient for FC at least

glacial crag Apr 19, 2024, 11:40 PM

#

yeah for sure

plain solar Apr 19, 2024, 11:41 PM

#

glacial crag downloading a neoforge jar through SPL would be quite cursed

wait, that's brilliant

haughty copper Apr 19, 2024, 11:41 PM

#

hm?

#

That's the entire point

glacial crag Apr 19, 2024, 11:41 PM

#

hehehe

haughty copper Apr 19, 2024, 11:41 PM

#

Of it being complicated

glacial crag Apr 19, 2024, 11:41 PM

#

I just hadn't thought of that potential use case - but it is something that would be pretty cool

haughty copper Apr 19, 2024, 11:41 PM

#

Since initially I'd just have included earlydisplay in the NF Jar

#

and had a very very small boot-loader in front of it

#

but loading up SPL will be a long-running thing 😐

#

and what previously was a simple "search for boot jars" now becomes an annoying iterative process

#

since SPL would have to contribute the contents of the neoforge jar after the fact

glacial crag Apr 19, 2024, 11:43 PM

#

hmmm yes you are right

haughty copper Apr 19, 2024, 11:48 PM

#

AHem

#

ModJarURLHandler and modjar://

#

is that even wired up anymore?

steel egret Apr 19, 2024, 11:55 PM

#

What is in 5 days?

haughty copper Apr 19, 2024, 11:55 PM

#

1.20.5

steel egret Apr 19, 2024, 11:56 PM

#

Ah

#

Makes sense

haughty copper Apr 20, 2024, 12:38 AM

#

Oh oops. Emitting a warning for every "plain" jar file also applies to all of the jars on the classpath

#

glacial crag Apr 20, 2024, 7:23 AM

#

haughty copper is that even wired up anymore?

I don't think so

chrome mortar Apr 20, 2024, 8:08 AM

#

haughty copper but ModValidator#validateFiles seems to be the right place for it

most validation is in the mod info ctors

glacial crag Apr 20, 2024, 8:11 AM

#

haughty copper Oh oops. Emitting a warning for every "plain" jar file also applies to all of th...

That's why I suggested having a way to mark some jars as "not required"

#

Potentially just at the locator level

#

OR we ignore jars that are in the legacy CP

haughty copper Apr 20, 2024, 8:58 AM

#

} else if (candidate.getPrimaryPath().startsWith(FMLLoader.getGamePath())) {

#

I'll just do it for stuff in the gamedir

glacial crag Apr 20, 2024, 9:00 AM

#

that's good enough I think

#

maybe just mention that this is a workaround for the lack of a better solution in some comment 😄

haughty copper Apr 20, 2024, 9:19 AM

#

already did

#

I still think that these warnings produced by our config system are misleading: [11:03:10] [main/WARN] [ne.ne.fm.lo.FMLConfig/CORE]: Configuration file C:\Users\SEBAST~1\AppData\Local\Temp\gameDir16569562345923357093\config\fml.toml is not correct. Correcting

#

This is when it does not exist, I believe

#

Oh that just means our default config file is missing keys

glacial crag Apr 20, 2024, 9:34 AM

#

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

GitHub

Move `EventBusSubscriber` to top-level and cleanup Bindings supplie...

Fixes #101.

#

the config system is next up on my FML todo list 😄

glacial crag Apr 20, 2024, 9:35 AM

#

glacial crag https://github.com/neoforged/FancyModLoader/pull/118

we might want to rebase-merge this one

errant yew Apr 20, 2024, 9:36 AM

#

do you want me to enable the rebase-merge option in anticipation of that

#

oh wait, it's already enabled

glacial crag Apr 20, 2024, 9:36 AM

#

usually it's just the merge-merge option that's disabled 😛

#

actually I can change that setting too thonk

errant yew Apr 20, 2024, 9:37 AM

#

thinkies

glacial crag Apr 20, 2024, 9:37 AM

#

I just enabled auto-merge hehe

#

do we have the required check for CI already?

errant yew Apr 20, 2024, 9:41 AM

#

nope, not on FML

haughty copper Apr 20, 2024, 9:44 AM

#

Gotta do a JiJ test case

#

and one for each FMLModType

errant yew Apr 20, 2024, 9:45 AM

#

check has been added

#

with the customary bypass on PRs for maintainers

chrome mortar Apr 20, 2024, 9:46 AM

#

glacial crag the config system is next up on my FML todo list 😄

thinkies do you want to use some of my work

#

also should i set up JCC on FML thinkies

haughty copper Apr 20, 2024, 9:46 AM

#

JCC?

errant yew Apr 20, 2024, 9:46 AM

#

jarcompatibilitychecker

haughty copper Apr 20, 2024, 9:46 AM

#

What does that do?

errant yew Apr 20, 2024, 9:46 AM

#

checks for (source/binary) compatibility between two JARs

haughty copper Apr 20, 2024, 9:46 AM

#

Ah interesting, sure why not

glad cobalt Apr 20, 2024, 9:46 AM

#

Right now that is not reasonable

#

But I think that we want to move to something like an API or implementation jar

errant yew Apr 20, 2024, 9:47 AM

#

https://github.com/neoforged/JarCompatibilityChecker for reference

GitHub

GitHub - neoforged/JarCompatibilityChecker: A tool that checks for ...

A tool that checks for binary compatibilities between two jars - neoforged/JarCompatibilityChecker

glad cobalt Apr 20, 2024, 9:47 AM

#

And then I think JCC on something like that would really make sense

haughty copper Apr 20, 2024, 9:47 AM

#

var plainModJar = installation.getModsFolder().resolve("plainmod.jar");
assertThat(result.issues()).containsOnly(ModLoadingIssue.warning(
        "fml.modloading.brokenfile", plainModJar
).withAffectedPath(plainModJar));

That is a bearable way to assert mod loading warnings / errors imho

errant yew Apr 20, 2024, 9:47 AM

#

is that a hamcrest matcher

haughty copper Apr 20, 2024, 9:47 AM

#

No, AssertJ

chrome mortar Apr 20, 2024, 9:47 AM

#

glad cobalt Right now that is not reasonable

it's less about knowing if the chags re breaking, that's not a concern right now and for a while

errant yew Apr 20, 2024, 9:48 AM

#

dammit, that was my first thought

chrome mortar Apr 20, 2024, 9:48 AM

#

it's more about documenting the breaks

glad cobalt Apr 20, 2024, 9:48 AM

#

In reality, I would like to make it clear: FML rarely changed in the past, and if it did it was breaking in 99% of the time.....

#

And I think once these changes have gone through

#

We will just drop back into that pattern

#

Look maty, if you want to spend time on it, be my guest

#

But it won't really get you any new information

haughty copper Apr 20, 2024, 9:49 AM

#

If it's "free" uh why not. but I wouldn't invest time into it right now either

#

Since we already know we're reaaaaaaaaaaly breaking API right now

#

@glacial crag That stuff in the Bindings static initializer

#

Uuhgh

glacial crag Apr 20, 2024, 9:50 AM

#

what?

glad cobalt Apr 20, 2024, 9:50 AM

#

That is kind of my point sharte

#

Like this is an opensource project

#

He is a maintainer

haughty copper Apr 20, 2024, 9:50 AM

#

final var providers = ServiceLoaderUtils.streamServiceLoader(() -> ServiceLoader.load(FMLLoader.getGameLayer(), IBindingsProvider.class), sce -> {}).toList();

glacial crag Apr 20, 2024, 9:50 AM

#

dude it was already creating the instance in the static initializer 😛

glad cobalt Apr 20, 2024, 9:50 AM

#

As such if he thinks that is a valueable amount of time

#

Then yeah

haughty copper Apr 20, 2024, 9:50 AM

#

Yes I know

#

I am not talking about your change, I am talking about the Gesamtsituation.

glacial crag Apr 20, 2024, 9:50 AM

#

hehehe

#

I might just remove the ServiceLoaderUtils call because it's just pointless

chrome mortar Apr 20, 2024, 9:51 AM

#

haughty copper If it's "free" uh why not. but I wouldn't invest time into it right now either

all infra was set up a couple of weeks ago 😛 it's a matter of generating the actions

errant yew Apr 20, 2024, 9:51 AM

#

"Gesamtsituation" what

haughty copper Apr 20, 2024, 9:51 AM

#

It's exactly what it sounds like, sci 😛

chrome mortar Apr 20, 2024, 9:51 AM

#

i read "caesar situation"

haughty copper Apr 20, 2024, 9:52 AM

#

Possibly 😄

glad cobalt Apr 20, 2024, 9:52 AM

#

😛

glacial crag Apr 20, 2024, 9:52 AM

#

previously all of that code was in the same source tree as neoforge

glad cobalt Apr 20, 2024, 9:52 AM

#

The Binding stuff is to jump classloaders

haughty copper Apr 20, 2024, 9:52 AM

#

@glacial crag My issue with it is:

glad cobalt Apr 20, 2024, 9:52 AM

#

If I remember properly

haughty copper Apr 20, 2024, 9:52 AM

#

Yeah I know Orion

glacial crag Apr 20, 2024, 9:52 AM

#

it's just to be able to reference neoforge from fml

glad cobalt Apr 20, 2024, 9:52 AM

#

But not sure if it is still needed

haughty copper Apr 20, 2024, 9:52 AM

#

It's more or less to access the translation system in MC

#

before MC is loaded

glad cobalt Apr 20, 2024, 9:52 AM

#

Yeah

glacial crag Apr 20, 2024, 9:52 AM

#

no it's after MC is loaded 😛

glad cobalt Apr 20, 2024, 9:52 AM

#

IMHO, We really should not be doing that

#

Anything before MC is loaded

haughty copper Apr 20, 2024, 9:52 AM

#

Well, yes but statically you can't compile against it, obviously

glad cobalt Apr 20, 2024, 9:52 AM

#

Should just be english

haughty copper Apr 20, 2024, 9:52 AM

#

since you don't have it at compiletime

glad cobalt Apr 20, 2024, 9:53 AM

#

Once translation is bootstrapped then we can switch

#

If we want to

glacial crag Apr 20, 2024, 9:53 AM

#

haughty copper since you don't have it at compiletime

yes that's the point hehe

haughty copper Apr 20, 2024, 9:53 AM

#

We should have the translations in FML, but currently we probably use standard MC rendering in earlydisplay to render the error screen

#

which is why the FML translations are in NF

#

Which is a whole mess of its own

#

Anyway, that is not a 1.20.5 change

#

But: What would be nice is to be able to know if Bindings are available or not

#

Currently it'll just blow up in your face unrecoverably if you try to call any of it before the game layer is built

glad cobalt Apr 20, 2024, 9:54 AM

#

haughty copper We should have the translations in FML, but currently we probably use standard M...

Yeah

glacial crag Apr 20, 2024, 9:55 AM

#

haughty copper Currently it'll just blow up in your face unrecoverably if you try to call any o...

which is sort of correct?

haughty copper Apr 20, 2024, 9:55 AM

#

You cannot do:

#

if (Bindings.areAvailable()) {
   print nice translated message using Bindings 
} else {
 print translation key + args
}

glacial crag Apr 20, 2024, 9:55 AM

#

well the thing is, we never want to translate when printing to the log IMO

chrome mortar Apr 20, 2024, 9:55 AM

#

chrome mortar all infra was set up a couple of weeks ago 😛 it's a matter of generating the ac...

huh, it was a month ago thinkies

haughty copper Apr 20, 2024, 9:56 AM

#

That was more a general observation 😄

#

It's just a minefield

glacial crag Apr 20, 2024, 9:56 AM

#

yes I know

haughty copper Apr 20, 2024, 9:56 AM

#

But don't take this as a comment on your PR in any case

#

We'll not fix that now. I hope we'll be able to get the locator stuff done though, the unified modelling of "mod loading issues" should help us build a better error reporting system E2E

glacial crag Apr 20, 2024, 9:56 AM

#

yes hopefully

haughty copper Apr 20, 2024, 9:57 AM

#

What I do not quite get: If we show translated messages ONLY using Minecraft rendering, why do we access translations at all in FML. I have to look into that.

glacial crag Apr 20, 2024, 9:58 AM

#

#

that's the only use of stripControlCodes and of course it's called by neoforge 😛

haughty copper Apr 20, 2024, 9:58 AM

#

Yeah that's what I meant

#

Some of that code seems like a loop-the-loop

errant yew Apr 20, 2024, 9:59 AM

#

the overengineer strikes again /jk

haughty copper Apr 20, 2024, 9:59 AM

#

NeoForge implements a ServiceLoader interface that FML looks up. And the only transitive use is by NeoForge itself through these methods?

glacial crag Apr 20, 2024, 9:59 AM

#

for that method yes

chrome mortar Apr 20, 2024, 9:59 AM

#

errant yew ~~the overengineer strikes again **/jk**~~

~~common pattern~~

glad cobalt Apr 20, 2024, 9:59 AM

#

haughty copper What I do not quite get: If we show translated messages ONLY using Minecraft ren...

Legacy

#

We needed that in the past

haughty copper Apr 20, 2024, 9:59 AM

#

Well, leave it. We'll need to be able to show translated messages without MC or NF if we go ahead with NF-as-a-mod

glad cobalt Apr 20, 2024, 10:00 AM

#

Because we could not directly do the rendering

#

We have different error screen tech now

haughty copper Apr 20, 2024, 10:00 AM

#

Nah Orion, what I mean is: FML could just have exposed the translation key + args to NF

glad cobalt Apr 20, 2024, 10:00 AM

#

And I am not sure it is still needed

glacial crag Apr 20, 2024, 10:00 AM

#

interestingly parseMessage is used in the exception message

haughty copper Apr 20, 2024, 10:00 AM

#

Yes I told you 😄

#

formatToString() is called by NF

glacial crag Apr 20, 2024, 10:01 AM

#

also by FML in getMessage

haughty copper Apr 20, 2024, 10:01 AM

#

Wait what

#

#

I thought this entire dance was to use a translation component oO

#

If we just retrieve strings, I really don't get why we have the translations in NF

glacial crag Apr 20, 2024, 10:01 AM

#

it's legacy code that's older than components

#

but it does call into MC's locale functions

haughty copper Apr 20, 2024, 10:02 AM

#

Yeah okay okay I'll stop 😄

glacial crag Apr 20, 2024, 10:02 AM

#

look at I18nExtension in NF if you want to get a heart attack

haughty copper Apr 20, 2024, 10:02 AM

#

ServiceLoaderUtils.streamServiceLoader does some error checks you have stripped with your code tech 😛

chrome mortar Apr 20, 2024, 10:02 AM

#

don't die pls, still need you to finish the FML PR

#

😛

haughty copper Apr 20, 2024, 10:03 AM

#

But you can do that Tech, I have an FML-local SLU in the PR

glacial crag Apr 20, 2024, 10:03 AM

#

haughty copper `ServiceLoaderUtils.streamServiceLoader` does some error checks you have strippe...

yes and I don't care about them 😛

#

I want the exception to be propagated, not silently caught

haughty copper Apr 20, 2024, 10:03 AM

#

Yeah in one-provider-is-allowed cases that's fine I suppose

glacial crag Apr 20, 2024, 10:04 AM

#

even then, it's not an error that's supposed to happen IMO

haughty copper Apr 20, 2024, 10:04 AM

#

The SLU util in FML just prints the summary or which implements are registered for a service which is nice if you can have 3rd party implementations

glacial crag Apr 20, 2024, 10:04 AM

#

silently catching it is really bad practice

haughty copper Apr 20, 2024, 10:04 AM

#

And it tries to identify the jar the service came from

glacial crag Apr 20, 2024, 10:04 AM

#

ok that actually sounds useful

haughty copper Apr 20, 2024, 10:04 AM

#

I think one of the existing impls did that too, no idea which one

#

Okay, how do I set up a minimal JIJ test case 😐

#

I need to produce the JIJ metadata in my test i suppose

glacial crag Apr 20, 2024, 10:05 AM

#

yes

#

it's very simple though

glad cobalt Apr 20, 2024, 10:05 AM

#

There is supposed to be a test jar somewhere

#

Either in FML

#

Or in JiJ

#

If I Remember correctly

#

Or in NF

#

Siude question: I am trying to force Javadoc to error if something is missing documentation

#

Does anybody know how to do this?

haughty copper Apr 20, 2024, 10:07 AM

#

Oof. I think I only ever had third party tools do that check

glad cobalt Apr 20, 2024, 10:07 AM

#

Ah there is

#

-Werror 😄

#

Great

#

Now to figure out how to tell gradle that

#

XD

#

Bingo 😄

#

Needs a gradle hack but works instnatly

haughty copper Apr 20, 2024, 10:12 AM

#

Orion!

#

Why does JIJ MetadataIOHandler have toInputStream but no toByteArray 😄

glad cobalt Apr 20, 2024, 10:12 AM

#

Ehm, was not needed XD

#

I whipped up JIJ against lexes whishes mostly, because I wanted a proper system that was able to deal with mixins in inner jars etc

#

So I tailored it originally to what I needed, and what was needed for the impl

#

I always said, if we need more, then add to it

haughty copper Apr 20, 2024, 10:13 AM

#

I mean this works 😅

byte[] content;
try (var in = MetadataIOHandler.toInputStream(metadata)) {
     content = in.readAllBytes();
} catch (IOException e) {
    throw new UncheckedIOException(e);
}

glad cobalt Apr 20, 2024, 10:13 AM

#

Make a ticket

#

I will address it later today

#

And get you a nice method

chrome mortar Apr 20, 2024, 10:13 AM

#

haughty copper I mean this works 😅 ```java byte[] content; try (var in = MetadataIOHandler.to...

switch with exceptions abolbsad

#

now i can't wait for it hahayes

haughty copper Apr 20, 2024, 10:14 AM

#

This is just a test anyway and it's nicely wrapped up 😄

#

Project Amber is doing some good stuff for java, really

#

public IdentifiableContent createJijMetadata(ContainedJarMetadata... containedJars) {
    Metadata metadata = new Metadata(Arrays.asList(containedJars));
    byte[] content;
    try (var in = MetadataIOHandler.toInputStream(metadata)) {
        content = in.readAllBytes();
    } catch (IOException e) {
        throw new UncheckedIOException(e);
    }
    return new IdentifiableContent("JIJ_METADATA", Constants.CONTAINED_JARS_METADATA_PATH, content);
}

chrome mortar Apr 20, 2024, 10:14 AM

#

is project amber the switchification of the language

haughty copper Apr 20, 2024, 10:14 AM

#

They do all those nice QoL things under project amber

chrome mortar Apr 20, 2024, 10:14 AM

#

so yes

#

😛

haughty copper Apr 20, 2024, 10:15 AM

#

We probably just ignore isObfuscated on the JIJ metadata, right?

#

Uuuuuh Caused by: java.nio.file.ProviderNotFoundException: Provider "jij" not found

glacial crag Apr 20, 2024, 10:17 AM

#

heh

glad cobalt Apr 20, 2024, 10:17 AM

#

haughty copper We probably just ignore isObfuscated on the JIJ metadata, right?

Yep

glacial crag Apr 20, 2024, 10:17 AM

#

it likely loads via SJH's url provider wrapper

glad cobalt Apr 20, 2024, 10:18 AM

#

haughty copper Uuuuuh `Caused by: java.nio.file.ProviderNotFoundException: Provider "jij" not f...

You likely loaded the FileSystems to early

#

FileSystemProviders are loaded once by the system

glacial crag Apr 20, 2024, 10:18 AM

#

see ModularURLHandler

glad cobalt Apr 20, 2024, 10:18 AM

#

So if you trigger that too early you can get into trouble

#

If I remember correctly

#

There was a way to reset that though, somewhere

haughty copper Apr 20, 2024, 10:19 AM

#

Yeah I'll have to debug that, IDK what too early means 😅 I probably forgot to call something

#

From my test

glacial crag Apr 20, 2024, 10:19 AM

#

there is no jij filesystem anymore right?

glad cobalt Apr 20, 2024, 10:19 AM

#

Yes there is

#

There has to be

#

Mixin and Classloaders won't work without it

glacial crag Apr 20, 2024, 10:19 AM

#

#

I think it's just unionfs

glad cobalt Apr 20, 2024, 10:19 AM

#

No

#

There is an actual JiJFS

#

#

It is known as LayeredZipFileSystemProvider

haughty copper Apr 20, 2024, 10:20 AM

#

Is FML missing a dep on that

glad cobalt Apr 20, 2024, 10:21 AM

#

I will check

haughty copper Apr 20, 2024, 10:21 AM

#

You don't have to, I am already in the project anyway

glacial crag Apr 20, 2024, 10:21 AM

#

FYI neoforge has it as a moduleOnly dep

glad cobalt Apr 20, 2024, 10:21 AM

#

I think that is fine

haughty copper Apr 20, 2024, 10:21 AM

#

yeah

glacial crag Apr 20, 2024, 10:21 AM

#

it needs to be in the boot module layer

glad cobalt Apr 20, 2024, 10:21 AM

#

It was always loaded via ServiceLocators

#

And yeah it has to be on the boot layer

#

Because as I said FileSystemProviders are discovered once by the JVM

#

And then never again

#

In particularly: FileSystemProvider#loadInstalledProviders()

glacial crag Apr 20, 2024, 10:22 AM

#

intellij doesn't see the dependency on it... 😐

haughty copper Apr 20, 2024, 10:23 AM

#

I think you specifically do not have to use the JIJ filesystem with JIJ

glacial crag Apr 20, 2024, 10:23 AM

#

glad cobalt Apr 20, 2024, 10:23 AM

#

haughty copper I think you specifically do not *have* to use the JIJ filesystem with JIJ

No you do....

glacial crag Apr 20, 2024, 10:23 AM

#

glacial crag

you can just pass a different classloader

haughty copper Apr 20, 2024, 10:23 AM

#

Well I had a prototype that copied the file out in the sourceProducer instead 😄

glad cobalt Apr 20, 2024, 10:24 AM

#

glacial crag you can just pass a different classloader

That looks like new

#

I think at least

#

Unsure

glacial crag Apr 20, 2024, 10:24 AM

#

I don't think so

glad cobalt Apr 20, 2024, 10:24 AM

#

glacial crag you can just pass a different classloader

Ah no

#

That does not work with URI

#

If you have a URI

#

That is turned into a Path

#

Via toPath

#

Then the classloader will be either null

glacial crag Apr 20, 2024, 10:24 AM

#

the problem is that the jdk will often call it without passing a specific classloader

glad cobalt Apr 20, 2024, 10:24 AM

#

Or the one for the bootlayer

#

Which is what Mixin and others do very frequently

#

Hence this needing to be on the bootlayer

haughty copper Apr 20, 2024, 10:25 AM

#

Yeah when turning URI->Path you can't pass a CL

#

(and neither would you want to as a downstream consumer)

#

It'd be nice if it tried the context CL, I suppose

glad cobalt Apr 20, 2024, 10:25 AM

#

Thsi whole FS thing is needed for URI support

#

If CL and Mixin worked with Path

#

It would not even be needed

#

And we could just use vanilla ZipFS

#

But since both use URIs

#

It still needs to be there

#

Sadly

haughty copper Apr 20, 2024, 10:26 AM

#

Anyhow, I'll just add a JIJ Filesystem dep for testRuntimeOnly

glad cobalt Apr 20, 2024, 10:26 AM

#

Yeah that should be fine

#

And should solve your issue

glacial crag Apr 20, 2024, 10:29 AM

#

NIO is just always so awkward once you start doing advanced stuff with it 😅

haughty copper Apr 20, 2024, 10:30 AM

#

Well we'll likely be tied into URI for all eternity

glad cobalt Apr 20, 2024, 10:30 AM

#

haughty copper Well we'll likely be tied into URI for all eternity

Yeah URIs won't go aways in Classloaders

haughty copper Apr 20, 2024, 10:30 AM

#

[12:30:11] [main/DEBUG] [ne.ne.fm.lo.mo.ModDiscoverer/SCAN]: Successfully Loaded 3 mods. Attempting to load dependencies...
[12:30:11] [main/DEBUG] [ne.ne.fm.lo.mo.ModDiscoverer/SCAN]: Trying locator net.neoforged.fml.loading.moddiscovery.locators.JarInJarDependencyLocator@38eb0f4d
[12:30:11] [main/INFO] [ne.ne.fm.lo.mo.lo.JarInJarDependencyLocator/]: Found 1 dependencies adding them to mods collection

#

Okay, it's "alive" 😄

glacial crag Apr 20, 2024, 10:30 AM

#

very nice

haughty copper Apr 20, 2024, 10:31 AM

#

the locator toString is still weird. We call toString on locators a bunch,but still have an additional name method

#

#

concern

#

That wasn't me 😄

glacial crag Apr 20, 2024, 10:32 AM

#

was probably maty

#

I don't think he had that wired up yet

chrome mortar Apr 20, 2024, 10:34 AM

#

i blame tech

haughty copper Apr 20, 2024, 10:34 AM

#

Hm

#

I'd have to check how JIJ behaves in the error case

#

[12:46:09] [main/ERROR] [ne.ne.fm.lo.mo.ModInfo/FATAL]: Invalid modId found in file - embedded-mod does not match the standard: ^[a-z][a-z0-9_]{1,63}$
[12:46:09] [main/ERROR] [ne.ne.fm.lo.mo.lo.JarInJarDependencyLocator/]: Failed to load mod file META-INF\jarjar\embedded-mod-1.0.jar from jijmod.jar
😄
Learning about JIJ more and more 😄

prisma elkBOT Apr 20, 2024, 10:48 AM

#

FML has different regexes for the different mod metadata:

Mod ID: [a-z][a-z0-9_]{1,63} (- is not a valid character as the mod ID determines the Java module name, and - isn't a valid character in module names)
Namespace: [a-z][a-z0-9_.-]{1,63}
Mod Version: \d+.* (versions must start with a digit due to Java module limitations)

haughty copper Apr 20, 2024, 10:48 AM

#

Yeah I didnt think it would validate the mod id

chrome mortar Apr 20, 2024, 10:48 AM

#

but it's not JIJ

#

it's the mod info ctor thinkies

haughty copper Apr 20, 2024, 10:48 AM

#

Oh

#

Yeah that's what I was afraid of the, error handling leads to a hard crash in that spot 😄

glad cobalt Apr 20, 2024, 11:14 AM

#

JIJ Requests mod info data about all candidates

#

Even if it does not decide to load it in the end after all

haughty copper Apr 20, 2024, 11:51 AM

#

I need to check if I can pass it a list<T> and get back a list <U>

#

Then I can have the source provider return a data result like type

glacial crag Apr 20, 2024, 11:57 AM

#

I'm still annoyed by the generics in JarJarSelector

haughty copper Apr 20, 2024, 11:57 AM

#

I like that it's agnostic to what it builds

glad cobalt Apr 20, 2024, 11:58 AM

#

Yeah that is the point

haughty copper Apr 20, 2024, 11:58 AM

#

Which allows us flexibility down the line

glad cobalt Apr 20, 2024, 11:58 AM

#

Because it was still at one point under consideration that FG6 would flatten the zips

#

And then it needed to be able to run the selector on its own

glacial crag Apr 20, 2024, 11:58 AM

#

haughty copper Which allows us flexibility down the line

well, only to the extent where we don't have to refactor the generic class...

glad cobalt Apr 20, 2024, 11:59 AM

#

Thonk is indeed the correct reaction

haughty copper Apr 20, 2024, 11:59 AM

#

I gotta check if we can have it continue processing on errors

glad cobalt Apr 20, 2024, 11:59 AM

#

You may gues three times whoes idea that was

#

And why it does not exist

glacial crag Apr 20, 2024, 11:59 AM

#

why would one flatten the zips

glad cobalt Apr 20, 2024, 12:00 PM

#

deduplication

#

Mostly

#

So that it shrinks the jar size

glacial crag Apr 20, 2024, 12:00 PM

#

thonk

glad cobalt Apr 20, 2024, 12:01 PM

#

Tja tech

#

I thought you would have expected that

#

You have been around this modding block once or twice already XD

glacial crag Apr 20, 2024, 12:01 PM

#

well not all bad ideas are from Lex 😛

glad cobalt Apr 20, 2024, 12:02 PM

#

XD

#

True

#

And this is not bad perse

glacial crag Apr 20, 2024, 12:02 PM

#

this doesn't sound bad, it just sounds meh

glad cobalt Apr 20, 2024, 12:02 PM

#

But it is also not very great

glacial crag Apr 20, 2024, 12:02 PM

#

yeah

glad cobalt Apr 20, 2024, 12:02 PM

#

The idea was that 1) we could shrink the jars, and 2) we would not need recursive JarJar

#

But 1) really does not matter these days

#

And 2) Did not reduce the complexity

haughty copper Apr 20, 2024, 12:03 PM

#

I noticed you can specify a folder in the jar as relative path and it does not complain hehe

glad cobalt Apr 20, 2024, 12:04 PM

#

No it does not 😄

#

You can package them literally anywhere

#

And reference them in the metadata

#

Only the metadat has a fixed location

#

And the rest is convention

#

Which is by design

haughty copper Apr 20, 2024, 12:05 PM

#

Well I mean not even a file in the jar heh

glad cobalt Apr 20, 2024, 12:05 PM

#

Yeah that also works

#

Because it is just a path

haughty copper Apr 20, 2024, 12:05 PM

#

Yeah

glad cobalt Apr 20, 2024, 12:05 PM

#

JarJar is really in that regard a dump piece of tech

#

Its intelligence lies in two thigns:

#

Its FS system

#

Its negotiator

#

The rest is just a pure dump locator

#

that just tells FML to fucking load shit

chrome mortar Apr 20, 2024, 1:38 PM

#

@glacial crag https://github.com/neoforged/FancyModLoader/pull/118/files#diff-654401a66f316514958691d6ea49201b940d748d6f460e2b24022c11c3bfd444R11 i would start annotating things with @Internal thinkies

glacial crag Apr 20, 2024, 1:47 PM

#

true that can be annotated

#

there you go

chrome mortar Apr 20, 2024, 4:10 PM

#

@glacial crag maybe it's also worth adding a code snipped in the docs of EventBusSubscriber so that it's clear that people need to use static methods w/ SubscribeEvent

haughty copper Apr 20, 2024, 4:24 PM

#

Which cases for JiJ should I account for

#

I have:

embedded mod ("META-INF/jarjar/embedded_mod-1.0.jar", SimulatedInstallation.createModsToml("embeddedmod", "1.0"))
embedded service ("META-INF/jarjar/embedded_service-1.0.jar", SimulatedInstallation.createManifest("SERVICE_MANIFEST", Map.of("FMLModType", "LIBRARY"))
embedded game library ("META-INF/jarjar/embedded_gamelib-1.0.jar", SimulatedInstallation.createManifest("GAMELIB_MANIFEST", Map.of("FMLModType", "GAMELIBRARY")))
embedded plain jar with no manifest ("META-INF/jarjar/embedded_lib-1.0.jar")

glad cobalt Apr 20, 2024, 4:26 PM

#

Those are the 4 I know of

#

In the future I really also want embedded ML plugin

#

But that requires a major refactor anyway

#

So......

#

Would be funny:

haughty copper Apr 20, 2024, 4:26 PM

#

The assertions essentially are:

assertThat(result.gameLayerModules()).containsOnlyKeys("minecraft", "embeddedmod", "embedded.gamelib", "jijmod", "neoforge");
assertThat(result.pluginLayerModules()).containsOnlyKeys("embedded.lib", "embedded.service");
assertThat(result.loadedMods()).containsOnlyKeys("minecraft", "neoforge", "embeddedmod", "jijmod");

glad cobalt Apr 20, 2024, 4:26 PM

#

Packaging all our libraries into the neoforge jar

#

And go home

#

Including mixin

haughty copper Apr 20, 2024, 4:27 PM

#

That's at least my idea but a half baked idea without a prototype is not worth much

#

So I am trying to get this out the door so I can iron out some prototype code 😄

glad cobalt Apr 20, 2024, 4:27 PM

#

That only works if you move all the locator code to one place

#

And that means FML needs to be installed

haughty copper Apr 20, 2024, 4:27 PM

#

I am thinking along the lines of a boot locator that replaces BSL

glad cobalt Apr 20, 2024, 4:27 PM

#

Instead of loaded from the NF jar

haughty copper Apr 20, 2024, 4:27 PM

#

and is a very lightweight loader that does know about mods/ but that's it

glad cobalt Apr 20, 2024, 4:28 PM

#

haughty copper and is a very lightweight loader that *does* know about `mods/` but that's it

That is what ML does right now

haughty copper Apr 20, 2024, 4:28 PM

#

Yes but it does more than that

#

My current idea would not have any transform code in the boot loader

#

no asm, nothing like that

glad cobalt Apr 20, 2024, 4:28 PM

#

But that still means that you can not distribute NF via SPL for example, or Mixin via the NF jar or any of the stuff like that

glad cobalt Apr 20, 2024, 4:28 PM

#

haughty copper My current idea would not have any transform code in the boot loader

It will always need to do that

#

Unless you put it one layer up

haughty copper Apr 20, 2024, 4:28 PM

#

You could, since all that thing needs to do is scan mods/ for boot layer content and build a boot layer to continue

glad cobalt Apr 20, 2024, 4:29 PM

#

Yeah, but that is not really practical.......

haughty copper Apr 20, 2024, 4:29 PM

#

So here's a provocative thought

#

inverse the control between FML and ML

glad cobalt Apr 20, 2024, 4:29 PM

#

Yeah I have that in design phase already

#

What I thought of was splitting FML

haughty copper Apr 20, 2024, 4:29 PM

#

Since we do not need to actually apply any transforms until the very end of FMLs startup code

glad cobalt Apr 20, 2024, 4:29 PM

#

Move most parts of it into NF

#

leave the locators and the connection to ML + the instantiation with language loaders in it

#

And then just install it with ML on the boot path

#

And go home

#

The amount of changes to that logic will be so tiny

#

And thanks to service loaders we could do locators recursively

#

And load them one at a time

haughty copper Apr 20, 2024, 4:30 PM

#

but here's the thing: have a boot loader that knows absolutly nothing about transforms, which is only responsible for essentially starting the iterative class-loading process of grabbing bootup code from our "blessed" mod jar

#

yeah we need an iterative locator process to support SPL also supplying the NF jar

glad cobalt Apr 20, 2024, 4:30 PM

#

Yeah that is the UX dream

#

But that means if you want to support that

haughty copper Apr 20, 2024, 4:30 PM

#

that aspect makes it hella complicated, but to fully go that route, we need it

glad cobalt Apr 20, 2024, 4:31 PM

#

You get into problems with mods wanting to modify the ELS

#

Like embeddium

#

embeddium could be gated behind SPL

haughty copper Apr 20, 2024, 4:31 PM

#

ELS?

#

Ah early loading

glad cobalt Apr 20, 2024, 4:31 PM

#

Early Loading Screen

#

But SPL wants/needs an ELS

haughty copper Apr 20, 2024, 4:31 PM

#

Yes, we had that discussion yeah. Embeddium already said he asks people to disable EL via config so he can hook it later

#

Yeah I had the same thought

glad cobalt Apr 20, 2024, 4:32 PM

#

So I have a design in my head

haughty copper Apr 20, 2024, 4:32 PM

#

There are some aspects we can only iron out through prototyping, really

glad cobalt Apr 20, 2024, 4:32 PM

#

That works

glacial crag Apr 20, 2024, 4:32 PM

#

FFS spotless destroys snippets...

haughty copper Apr 20, 2024, 4:32 PM

#

spotless is just /sigh

glad cobalt Apr 20, 2024, 4:32 PM

#

It would make ML so tiny

glacial crag Apr 20, 2024, 4:32 PM

#

it replaces @ by @

chrome mortar Apr 20, 2024, 4:32 PM

#

facepalm

haughty copper Apr 20, 2024, 4:32 PM

#

why is there nothing better in javaland

#

You could seriously just drop ML and rename it NFBL

glad cobalt Apr 20, 2024, 4:32 PM

#

And FML really modular, with basically just a tiny API that Locators and Language Loaders hook

haughty copper Apr 20, 2024, 4:32 PM

#

NeoForgeBootLoader 😄

glad cobalt Apr 20, 2024, 4:32 PM

#

And that is it

#

Nah

#

ML will need to contain the logic to build a transformable multi layered system

#

No need to refactor that

haughty copper Apr 20, 2024, 4:33 PM

#

I am envisioning a really tiny lib we install into launcher profiles that is already customized to us in that it knows of mods/

glad cobalt Apr 20, 2024, 4:33 PM

#

The libraries ML uses should not be touched by anybody anyway

haughty copper Apr 20, 2024, 4:33 PM

#

You pull in ML as al ib

#

*lib

#

in FML and package it into the NF jar

glad cobalt Apr 20, 2024, 4:33 PM

#

Yeah but that idea does not work with SPL

haughty copper Apr 20, 2024, 4:33 PM

#

I guarantee you

#

it does

glad cobalt Apr 20, 2024, 4:33 PM

#

Or at least an NF distro via SPL

haughty copper Apr 20, 2024, 4:33 PM

#

But I really need to pour it into code

glad cobalt Apr 20, 2024, 4:34 PM

#

Yeah we should eventually hop into a call together

#

And maybe just draw it out on paper

#

I think we are close to having the same idea

#

Just with some small tweaks

haughty copper Apr 20, 2024, 4:34 PM

#

And yeah, feature-wise the goal should obviously be:

SPL should be able to supply the NF jar (which requires iterative locator / layer building)
SPL needs to be able to show progress while it downloads mods

glad cobalt Apr 20, 2024, 4:34 PM

#

Yeah

#

With that

#

You could make amazing launchers

#

Which have like fully dynamic pack mangement build in into them

haughty copper Apr 20, 2024, 4:35 PM

#

SPL is niche though, we should acknowledge that

#

it's mostly for FC 😄

glad cobalt Apr 20, 2024, 4:35 PM

#

True

#

But imagine a modpack launcher

haughty copper Apr 20, 2024, 4:35 PM

#

Since it's such a security nightmare if you let the general population use it

glad cobalt Apr 20, 2024, 4:35 PM

#

That does not install anything at all

#

Just has that one locator

#

You start the game

haughty copper Apr 20, 2024, 4:35 PM

#

In general the "installer" becomes a joke anyway

glad cobalt Apr 20, 2024, 4:35 PM

#

And it sets up the entire instance

haughty copper Apr 20, 2024, 4:35 PM

#

since it just copies a version JSON over 😄

#

our current installer code essentially moves as an embed into the NF jar

glad cobalt Apr 20, 2024, 4:36 PM

#

Yeah

haughty copper Apr 20, 2024, 4:36 PM

#

we neeeeeeeeed some optimizations along the road for that to get the size down to a humane level, but I have some thoughts on that

#

we currently ship too many duplicate patch files

#

but we can trivially fix that

glad cobalt Apr 20, 2024, 4:36 PM

#

Luke has send my mind down a path

#

Using gradle metadata and dependency resoluttion

haughty copper Apr 20, 2024, 4:36 PM

#

one more for the TODO board:

glad cobalt Apr 20, 2024, 4:36 PM

#

And variant publishing....

haughty copper Apr 20, 2024, 4:36 PM

#

rethink if we even need a joined distribution in neoform

split wasp Apr 20, 2024, 4:36 PM

#

ohno what have I done

haughty copper Apr 20, 2024, 4:37 PM

#

if all we do is have onlyin's on classes

glad cobalt Apr 20, 2024, 4:37 PM

#

Your ideas on configurations and variant publishing

#

They are not bad, but still very rough

#

But I think we can even push them

glad cobalt Apr 20, 2024, 4:37 PM

#

haughty copper rethink if we even need a joined distribution in neoform

We don't, really. We only need a JOINED

#

Server and everything else can be yeeted for all I care

#

We distro the entire client anyway to the server

#

And stuff like that

#

So lets just have one type

haughty copper Apr 20, 2024, 4:38 PM

#

Hm?

#

glad cobalt Apr 20, 2024, 4:38 PM

#

Yeah

haughty copper Apr 20, 2024, 4:38 PM

#

This is from the installers data directory

glad cobalt Apr 20, 2024, 4:38 PM

#

Have it apply it XD

#

And you will see that the entire client pops up XD

haughty copper Apr 20, 2024, 4:38 PM

#

the EASIEST way of actually compressing that down is actually more stupid 😄

#

Since both these files

#

are just lzma compressed jar files

#

And they have significant overlap in their content

#

Uh @glad cobalt I think something is fucked about our server patchset. Or I am being stupid here.

#

#

Why does the server patch set contain client-side renderer patches in official names?

glad cobalt Apr 20, 2024, 4:44 PM

#

No that is by design

#

It patches back in all client classes

#

Including their onlyin annotations

haughty copper Apr 20, 2024, 4:44 PM

#

oO

glad cobalt Apr 20, 2024, 4:44 PM

#

And then the transformer sees them and throws the user-friendly exception

chrome mortar Apr 20, 2024, 4:45 PM

#

shartte that's why onlyin exists lol

glad cobalt Apr 20, 2024, 4:45 PM

#

It is the only way to 💯% know if the class is actually missing or somebody fucked up on the server

haughty copper Apr 20, 2024, 4:45 PM

#

I thought OnlyIn exists to be able to run a dedicated server without client classes in dev

#

where you are forced to have a joined distribution

glad cobalt Apr 20, 2024, 4:45 PM

#

Nah

haughty copper Apr 20, 2024, 4:45 PM

#

Because I'd personally just have removed OnlyIn in production to get some speed 👀

glad cobalt Apr 20, 2024, 4:46 PM

#

We have a joined distro literally everywhere

haughty copper Apr 20, 2024, 4:46 PM

#

No?

#

We don't

glad cobalt Apr 20, 2024, 4:46 PM

#

Yeah we do

#

The server and client jars once they are patched are identical

haughty copper Apr 20, 2024, 4:46 PM

#

#

Yes but the unpatched one isn't

glad cobalt Apr 20, 2024, 4:46 PM

#

The patches are generated from the same target

haughty copper Apr 20, 2024, 4:46 PM

#

So it's by our choice

glad cobalt Apr 20, 2024, 4:46 PM

#

Sure

#

But as I said

#

We run everywhere with a joined distro

glacial crag Apr 20, 2024, 4:47 PM

#

jesus

#

in some cases it makes sense for us to ship a vanilla client-only class on the server too

haughty copper Apr 20, 2024, 5:00 PM

#

Hrmpf Orion, I think the JIJ filesystem Path implementation doesn't have a nice tostring, or I am not sure how else to solve this

#

#

The root ("") being used as the SJH primary path leads to this semi-helpful ModFile#tostring:

#

Nested Mod File in Mod File: C:\Users\SEBAST~1\AppData\Local\Temp\gameDir13459412453034527518\mods\jijmod.jar

#

I added the nested part, which helps a little bit at least

#

Oh no this is a union filesystem wrapping a JIJ filesystem wrapping a union filesystem wrapping a windows path?

plain solar Apr 20, 2024, 5:07 PM

#

haughty copper SPL is niche though, we should acknowledge that

FWIW, there are many times when I wished for something like SPL when distributing custom modpacks to friends years ago, so I think it would have value if it were documented better and/or easier to setup

chrome mortar Apr 20, 2024, 5:13 PM

#

#builds message 502? hyperconcern

prisma elkBOT Apr 20, 2024, 5:13 PM

#

[Jump to referenced message](#builds message) in #builds

FancyModLoader

Build Failed

Version

3.0.17

Build Branch

main

Commit message

Move EventBusSubscriber to top-level and cleanup Bindings suppliers (#118)

Move EventBusSubscriber to top-level and cleanup Bindings suppliers
Inline I18nParser into IBindingsProvider
Rename FORGE bus to GAME bus
Add EBS snippet in javadoc

glacial crag Apr 20, 2024, 5:14 PM

#

let's try again lol

#

worked this time

#

https://tenor.com/view/chernobyl-not-great-not-terrible-average-gif-14312851

Tenor

haughty copper Apr 20, 2024, 5:21 PM

#

plain solar FWIW, there are many times when I wished for something like SPL when distributin...

The problem is hammering it into the users of the client-side that the server can send you whatever executable they want 😄

plain solar Apr 20, 2024, 5:21 PM

#

Is that any worse than people distributing ZIPs to each other?

#

I suppose SPL makes problematic software easier to mass-distribute

#

but at an individual level it is no worse than the server owner giving you the files manually

haughty copper Apr 20, 2024, 5:22 PM

#

plain solar Is that any worse than people distributing ZIPs to each other?

It's just that it's intransparent

#

And that they can change what they distribute without you knowing

glad cobalt Apr 20, 2024, 5:24 PM

#

I was eventually planning on adding like an update screen to it

#

That you can / need to click to update

#

But for now ELS is not that flexible

haughty copper Apr 20, 2024, 5:28 PM

#

Oh, picking up the license validation again: It should only do that for mods, right?

#

Since it just complained about a game library

#

So... I changed it to this instead, is that correct?

if (modFile.getType() == IModFile.Type.MOD && Strings.isNullOrEmpty(modFile.getModFileInfo().getLicense())) {
    issues.add(ModLoadingIssue.error("fml.modloading.missinglicense", modFile).withAffectedModFile(modFile));
}

chrome mortar Apr 20, 2024, 5:29 PM

#

why won't you just... move that check where the other checks are in the mod info ctor thinkies

haughty copper Apr 20, 2024, 5:30 PM

#

Hmmm

#

yes

#

Fine 😄

#FML Clean-up