New IEventListener changes cause Storage Drawers errors

[jchung01]

Your CleanroomMC Discord Username

ChaosStrikez

Cleanroom Version

0.3.2-alpha

Java Version

Java 21

Graphics Card Vendor

NVidia

Bug Report

The changes in #330 means the codepath taken for registering event handlers is slightly different. This surfaces a bug within Storage Drawers (SD) loading client-side classes on dedicated servers with the following flow:

1. Storage Drawers registers its Thaumcraft integration to the Forge event bus.
2. The class has a method signature referencing a client-only class BufferBuilder.
3. The optimized event handler class scanning as of Cleanroom 0.3.1-alpha means each method is scanned lazily in order, possibly skipping the call to Class#getDeclaredMethod().
4. In Cleanroom, the class and its method signatures will not cause class loading until the first getDeclaredMethod(), which is on a super-method.
   ** In Forge, the very first method scanned will call getDeclaredMethod(), which will immediately class-load BufferBuilder, detect, and throw an exception in SideTransformer, so the next step will not occur.
5. However, this super-method is only scanned after onDrawerPopulated(), so the event listener for onDrawerPopulated is registered, and then the sidedness exception is thrown.

Normally, this should all result in the server crashing, but SD swallows the exception, allowing the server to continue as normal besides logging the error. This is the extent to which any issues would arise in Forge. However, in Cleanroom, this results in SD's class in an odd state where the event subscriber was registered, but the fields weren't initialized. This will result in an error (possibly crash) every time the event is posted when storage drawers' items are updated, like so https://mclo.gs/cqQgXnY.

I can submit a PR to SD if necessary to prevent client-only class loading like so, but maybe some changes on Cleanroom's side as a safeguard would be good? Some options I can think of:

• Unconditionally force a call to Class#getDeclaredMethod() on the first method scan
• Have the first method scanned be one that is not a direct public instance/static method (i.e. a superclass method) to force class-loading
• Use getDeclaredPublicMethod() instead of getDeclaredMethod() as event listeners can only be public? (only fixes this specific case)
• Handle exceptions directly during EventBus#register() and unregister anything done during the call?
• Revert the class scanning changes

Mod List

Minimal mods:

• Fugue-0.18.5
• Baubles-1.12-1.5.2
• Chameleon-1.12-4.1.3
• StorageDrawers-1.12.2-5.5.3
• Thaumcraft-1.12.2-6.1.BETA26

Final Checklist

• I have searched the issues and haven't found a similar issue.
• I have read the readme and know that what is action build.
• I have installed Fugue and it didn't fix my issue.
• I have installed Scalar and it didn't fix my issue.
• I have switched my Forgelin and LibrarianLib to continuous versions(check readme for more detail).
• I am running a test build from Cleanroom Github Actions. (Or, if I've compiled it myself I plan to fix the issue)

[kappa-maintainer]

Try latest action build
Edit: I forgot that PR builds won't trigger MMC pack building, please replace the universal jar manually

[jchung01]

Yeah, I tested the PR build (#1004) and the errors don't seem to be thrown anymore on a dedicated server, so it should be back to Forge's behavior (integration error logged, but no further errors). I'll go ahead and PR to Storage Drawers still as it is technically their bug.