Skip to content

Fix TropicalFishData spawning incorrect type#7744

Merged
APickledWalrus merged 8 commits into
SkriptLang:dev/featurefrom
Fusezion:patch/fix-fish-entitydata
Apr 1, 2025
Merged

Fix TropicalFishData spawning incorrect type#7744
APickledWalrus merged 8 commits into
SkriptLang:dev/featurefrom
Fusezion:patch/fix-fish-entitydata

Conversation

@Fusezion
Copy link
Copy Markdown
Contributor

Description

This PR aims to fix an issue with the TropticalFIsh entity data always spawning the next fish in enum ordinal instead of the expected type.

I.e. Snooper always was Dasher, Spotty was always Flopper, ect...

The fix for this was to not do + 1 for ordinals and move "tropical fish" to the end of the code names list


Target Minecraft Versions: any
Requirements: none
Related Issues: none

Before

image

After

image

and just to show random still works when undefined
image

@Fusezion Fusezion requested a review from a team as a code owner March 24, 2025 04:05
@Fusezion Fusezion requested review from cheeezburga and erenkarakal and removed request for a team March 24, 2025 04:05
@Efnilite Efnilite added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Mar 24, 2025
@sovdeeth sovdeeth added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Apr 1, 2025
Copy link
Copy Markdown
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

Looks good. Is moving the generic pattern "tropical fish" to the end necessary though? Wouldn't patterns[matchedPattern - 1] also work? Not a major issue though, just curious.

@Fusezion
Copy link
Copy Markdown
Contributor Author

Fusezion commented Apr 1, 2025

Looks good. Is moving the generic pattern "tropical fish" to the end necessary though? Wouldn't patterns[matchedPattern - 1] also work? Not a major issue though, just curious.

In truth when I was making this pr, I knew next to nothing about the handling of entitydata, due to lack of javadocs and was also working on near zero hours of sleep. I could most likely swap this to being handled with match pattern -1.

However I believe I'd prefer holding that off for now as I wouldn't be able to test it, and I believe it's likely best for a future PR to change this to use a enum implementation due to safety concerns regarding ordinals.

@APickledWalrus APickledWalrus merged commit 49f6c35 into SkriptLang:dev/feature Apr 1, 2025
@Fusezion Fusezion deleted the patch/fix-fish-entitydata branch April 7, 2025 14:15
erenkarakal pushed a commit to erenkarakal/Skript that referenced this pull request Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants