Skip to content

Comments

Potentially fix Issue with MBT and CEu#1544

Merged
ALongStringOfNumbers merged 2 commits intoGregTechCEu:masterfrom
Tictim:mbt_fix
Mar 2, 2023
Merged

Potentially fix Issue with MBT and CEu#1544
ALongStringOfNumbers merged 2 commits intoGregTechCEu:masterfrom
Tictim:mbt_fix

Conversation

@Tictim
Copy link
Contributor

@Tictim Tictim commented Feb 27, 2023

What

This PR addresses an error observed inside Multiblock Tweaker's dev env.

Implementation Details

MultiblockControllerBase#structurePattern is a public mutable field. It is only populated when MultiblockControllerBase#reinitializeStructurePattern() is called. On default implementation of MultiblockControllerBase this field gets always populated while instantiating MultiblockInfoRecipeWrapper - specifically, in getMatchingShapes() method - so the code does not produce any error in base Gregtech.

But, since MBT overrides getMatchingShapes, the call to reinitializeStructurePattern gets bypassed; as a result, accessing structurePattern inside MultiblockInfoRecipeWrapper causes NPE.

This PR fixes the issue by checking structurePattern for manual reinitialization.

Outcome

Although this seems somewhat related to #1506 in my opinion, I have no concrete evidence suggesting whether this is the core cause of the issue.

Additional Information

This whole debugging session took place in MBT dev env, with pre-provided sample script. In-game tests beyond searching for items and recipes in JEI are not performed. If anyone wants to test this patch, it would be greatly appreciated.

@StarL0st
Copy link
Contributor

So that is the reason it threw a NPE when logging into a server, gonna build this and try it out later today

@Tictim
Copy link
Contributor Author

Tictim commented Feb 27, 2023

The initial issue this PR was supposed to fix was proven to be a separate issue unrelated to #1506.

The latest commit contains fix for another MBT compat issue, which MIGHT be the source of #1506, or https://github.com/tracer4b/nomi-ceu/issues/262, as this fix affects server-side class loading behavior. But I don't know. I feel like it kind of isn't. Still, an in-game test would be appreciated.

The issue, which observed in MBT dev env, seems to occur with this step.

  1. CT script loads gregtech.client.renderer.texture.Textures to access cube renderer registry.
  2. Textures class contains TextureAtlasSprite fields.
  3. Wait. That's illegal.

During the testing, marking each TextureAtlasSprite with SideOnly annotation seems to make the class loadable from server-side, thus solving the issue.

Similar issue with classloading might still be present with different classes associated, which would produce similar behavior to this error. In my testing the error occurred on this line:

.withBaseTexture(<cube_renderer:FROST_PROOF_CASING>)

and chat reported:

ERROR: [crafttweaker]: Error executing {[0:crafttweaker]: 02_custom_map_example.zs}: Lnet/minecraft/client/renderer/texture/TextureAtlasSprite;, caused by java.lang.NoClassDefFoundError: Lnet/minecraft/client/renderer/texture/TextureAtlasSprite;

@TechLord22 TechLord22 added the type: bug Something isn't working label Feb 27, 2023
@TechLord22 TechLord22 added this to the 2.5.3 milestone Feb 27, 2023
@Tictim
Copy link
Contributor Author

Tictim commented Feb 27, 2023

gregtech-1.12.2-2.5.2-beta.zip

Here is a build of this PR.

@StarL0st
Copy link
Contributor

Tested it in a server with that build, works with no errors

@ALongStringOfNumbers
Copy link
Contributor

Tested this on a server with the latest GCP, the null error described in the initial post was present on the server, which this PR fixed.

@ALongStringOfNumbers ALongStringOfNumbers changed the title Potentially fix Issue with MBT and CEu but differently Potentially fix Issue with MBT and CEu Mar 2, 2023
@ALongStringOfNumbers ALongStringOfNumbers merged commit 9e474b3 into GregTechCEu:master Mar 2, 2023
@Tictim Tictim deleted the mbt_fix branch March 3, 2023 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants