Fix reported bug around PHBE and illusionary dragons #182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Looking into this also revealed that liches suffered the same problem/bug, so they have been fixed as well. Additionally the spell descriptions were incorrect in that they implied you could only summon one dragon or lich per turn illusorily, but the code would actually allow you to summon up to the (incorrect) limit. Allowing this to take a number mimics the other illusory spells, and the max a unit can hold of the illusory objects mimics the max they can hold of the real objects. Additional to this, while reviewing all the summon code, I discovered that the spell description for Bird Lore 3 was incorrectly using the .mOut value for skeletons rather than eagles. The actual execution code was using the correct .mOut value, so the correct fix was to make the spell description correct to match reality. This does cause the snapshot turns to need to be updated since those reflected the wrong value in the spell description.
The MacOS (clang) compiler is stricter about static analysis than some of the others. It correctly detected that these variables were set and incremented but never read, so errored about them being unused. So, let's get rid of dead variables.
jt-traub
commented
May 2, 2024
| @@ -1062,17 +1062,13 @@ void ARegionList::SetFractalTerrain(ARegionArray *pArr) | |||
|
|
|||
| void ARegionList::NameRegions(ARegionArray *pArr) | |||
| { | |||
| int seed = 0; | |||
| int grown = 0; | |||
There was a problem hiding this comment.
All of the variables in this file being removed here are set and incremented but never actually used, so the MacOS X compiler (CLANG) complained about them since it's static analysis code is stronger than the standard GCC code.
artyomtrityak
approved these changes
May 2, 2024
valdisz
approved these changes
May 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Looking into this also revealed that liches suffered the same problem/bug, so they have been fixed as well. Additionally the
spell descriptions were incorrect in that they implied you could only summon one dragon or lich per turn illusorily, but the code would actually allow you to summon up to the (incorrect) limit. Allowing this to take a number mimics the other illusory spells, and the max a unit can hold of the illusory objects mimics the max they can hold of the real objects.
Note that the
create phantasmal undeadskill is disabled, so the lich bug never showed up in a live game but it's fixed now anyway.Additional to this, while reviewing all the summon code, I discovered that the spell description for Bird Lore 3 was incorrectly using the
.mOutvalue for skeletons rather than eagles. The actual execution code was using the correct.mOutvalue, so the correct fix was to make the spell description correct to match reality.This does cause the snapshot turns to need to be updated since those reflected the wrong value in the spell description.
It also revealed that the most recent version of CLANG (the MacOS compiler) detected unused variables which were set and incremented but never actually read, so they were in fact dead code. The second commit in this PR cleans that up to fix the Mac compile.