Skip to content
This repository has been archived by the owner on Mar 8, 2024. It is now read-only.

Add type check to LevelLightEngineMixin for compatibility with Create #111

Merged
merged 1 commit into from
Jan 1, 2022

Conversation

AeiouEnigma
Copy link
Contributor

@AeiouEnigma AeiouEnigma commented Dec 25, 2021

This PR fixes a ClassCastException crash which occurs when using Create's Ponder feature while Starlight is installed.

Create's dependency, Flywheel, has a VirtualEmptyBlockGetter class which serves to ensure that everywhere in Create's fake world has 0 block light and 15 sky light. To accomplish this, the class uses a custom LevelLightEngine, passing to the constructor a LightChunkGetter whose getLevel method returns the VirtualEmptyBlockGetter itself, which does not inherit from Level.

Because Starlight replaces the LevelLightEngine constructor and passes the provided LightChunkGetter to the StarLightInterface constructor, where it is assumed that any provided non-null LightChunkGetter will return a Level from its getLevel method, using the mods together results in a ClassCastException when StarLightInterface's constructor attempts to cast Flywheel's VirtualEmptyBlockGetter to a Level.

Specifically, this PR changes Starlight's LevelLightEngineMixin::construct to check whether the provided LightChunkGetter's getLevel method returns an instance of Level. If it does not, the chunkProvider argument in the call to the StarLightInterface constructor is replaced with a null argument. This change avoids the ClassCastException that would otherwise result from using Create with Starlight.

Closes #45

…er::getLevel returns a Level. Avoids a possible ClassCastException in StarLightInterface's constructor if a custom LightChunkGetter which does not return a Level from its getLevel is passed as an argument to the LevelLightEngine constructor.
Comment on lines +61 to +66
// avoid ClassCastException in cases where custom LightChunkGetters do not return a Level from getLevel()
if (chunkProvider.getLevel() instanceof Level) {
this.lightEngine = new StarLightInterface(chunkProvider, hasSkyLight, hasBlockLight, (LevelLightEngine) (Object) this);
} else {
this.lightEngine = new StarLightInterface(null, hasSkyLight, hasBlockLight, (LevelLightEngine) (Object) this);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend moving the instanceof check into the StarLightInterface constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the first fix I attempted, which resulted in a hang on world load.
image

Performing the check in LeveLightEngineMixin fixed everything without issue.

@AeiouEnigma
Copy link
Contributor Author

My aforementioned PR to Create has been merged (albeit manually). If this PR is also merged, Create and Starlight will at last be compatible.

@AeiouEnigma AeiouEnigma changed the title Add type check in LevelLightEngineMixin::construct to avoid potential ClassCastException Add type check for compatibility with Create Dec 27, 2021
@AeiouEnigma AeiouEnigma changed the title Add type check for compatibility with Create Add type check to LevelLightEngineMixin for compatibility with Create Dec 27, 2021
@Matthysse
Copy link

Hello! I still have a crash with Starlight and create with the StarLight version that AeiouEnigma shared in the last mentioned issue: (with Not enough crashes: https://crashy.net/WHlr6n9dlVa5sN9qHMf1)
(or here is the raw crash report https://bytebin.lucko.me/jXwpVXgMZU)
Tested without Optifine or Sodium

@AeiouEnigma
Copy link
Contributor Author

Hello! I still have a crash with Starlight and create with the StarLight version that AeiouEnigma shared in the last mentioned issue

Could you please share exactly what you tried to do that caused the crash?

Anything other than a ClassCastException when Pondering, at this point, would indicate that either I missed an important test case when making changes to Create, or that a necessary Create change was messed up when migrating some of those changes to Flywheel.

Most likely this issue means there's something off about the custom world height implementation in Create, but I'll need to know exactly what you were doing, preferably with a screenshot, to produce the crash in order to be sure.

@AeiouEnigma
Copy link
Contributor Author

AeiouEnigma commented Dec 30, 2021

@Matthysse please try this custom build of Create and confirm it fixes your issue.

@Matthysse
Copy link

@AeiouEnigma yes! your build resolves indeed the crash! thanks for the quick reaction!

@AeiouEnigma
Copy link
Contributor Author

AeiouEnigma commented Dec 30, 2021

The fix for that embarrassing error has been merged into Create.

Now this current PR to Starlight is really and truly the last piece of the compatibility puzzle.

@BugmanBugman
Copy link

Can confirm this fixed my crashes!

@Spottedleaf
Copy link
Member

Provided a Create dev can approve this, I will merge it

@PepperCode1
Copy link

As a Create developer, I approve this. ⭐

@Spottedleaf Spottedleaf merged commit d14adfc into PaperMC:fabric Jan 1, 2022
Spottedleaf pushed a commit that referenced this pull request Jan 1, 2022
…:getLevel returns a Level. (#111)

Avoids a possible ClassCastException in StarLightInterface's constructor if a custom LightChunkGetter which does not return a Level from its getLevel is passed as an argument to the LevelLightEngine constructor.
Done for Create compatibility
@AeiouEnigma
Copy link
Contributor Author

AeiouEnigma commented Jan 1, 2022

One last note: Starlight 1.0.1+ won't actually be fully compatible with Create until their next release.

@Spottedleaf
Copy link
Member

I will wait to push 1.0.1 until then, as I want to verify locally it works

@Spottedleaf
Copy link
Member

I've instead decided to release an alpha build so that users can get updated beforehand, the alpha build will be marked as release once there is a corresponding compatible build of create on CF

@AeiouEnigma
Copy link
Contributor Author

The new Create update just dropped, you can test v0.4c with Starlight 1.0.1 now

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Starlight Forge 1.0.0-RC + Create 0.3.1 conflict
6 participants