Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isolator crash fix revision #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Astatine5985
Copy link

@Astatine5985 Astatine5985 commented Dec 28, 2024

More consistent fix for the issue: GTNewHorizons/GT-New-Horizons-Modpack#17636

Problem: extracting genes in the isolator has a chance to cause NPE leading to a server crash

Cause: some logics were changed in IsolatorComponentLogic.java from this commit 39fe5d0
This results in the possibility of NPE by removing the try-catch statement.
Which was necessary because chromosomes objects in forestry. core. genetics Genome class contains null

Proposed Solution: check whether chromosomes is null before getting the allele, and loop until it isn't null

The previous fix, #55, returns right away when it comes across NPE instead of looping back to get a non-null gene, which makes the player wait for another cycle to get the isolated gene. Which is different from the original behavior.
This solution accounts for this so that the behavior of the isolator is consistent before being broken by the above commit.

@Dream-Master Dream-Master requested a review from a team December 28, 2024 12:19
IChromosomeType chromosome;
do {
chromosome = karyo[rand.nextInt(karyo.length)];
} while (individual.getGenome().getChromosomes()[chromosome.ordinal()] == null);
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible for all chromosomes to be null here, causing an infinite loop?

Copy link
Author

@Astatine5985 Astatine5985 Dec 28, 2024

Choose a reason for hiding this comment

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

It would be impossible for normal playthrough.
If all chromosomes are null it means that the bee doesn't even have a species on them. So, i wouldn't see anyone puting a blank bee in the isolator nor obtaining them.
Nonetheless, I think it would be nice to have a some kind of way to prevent an infinite loop. Incase someone puts a blank bee for debbuging purpose.

Copy link
Member

Choose a reason for hiding this comment

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

How about something like this?

List<IChromosomeType> chromosomes = Arrays.stream(karyo)
    .filter(Objects::nonNull)
    .collect(Collectors.toList());
if (chromosomes.isEmpty()) return;
IChromosomeType chromosome = chromosomes.get(rand.nextInt(chromosomes.size()));

Copy link
Author

Choose a reason for hiding this comment

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

karyo is a list that contains the chromosome types (SPECIES, PRODUCTION, HUMIDITY etc)

So, if we change this part of line 1
Arrays.stream(karyo)
to
Arrays.stream(individual.getGenome().getChromosomes())

And maybe change the last line too accordingly

So if we fix that part it would be a much better solution : D

  • I'm on my phone so excuse me for poor formatting.

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

Successfully merging this pull request may close these issues.

2 participants