Make the base_type of classed draconians their corresponding colors instead of randomly chosen. (Fix #4001)
#4181
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
code2828
changed the title
base_type of classed draconians their corresponding colors instead of randomly chosen.base_type of classed draconians their corresponding colors instead of randomly chosen. (Fix #4001)
Cgettys
reviewed
Jan 20, 2025
| @@ -1059,7 +1059,32 @@ static int _describe_monster(const string &key, const string &suffix, | |||
| // Might be better to show all possible combinations rather than picking | |||
| // one at random as this does? | |||
| if (mons_is_draconian_job(mon_num)) | |||
| base_type = random_draconian_monster_species(); | |||
| { | |||
| // Classed draconians have fixed colours since 0.28 | |||
There was a problem hiding this comment.
Might be nice to have a dictionary instead of a case statement.
And the bit in the comment about 0.28 doesn't quite make sense to me?
But overall, looks reasonable to me.
There was a problem hiding this comment.
Thanks for your review!
About the comment, The 0.28 release notes said that "Classed draconians have a fixed colour per job." so I figured that it might be good to note here since this is where the issue rose.
There was a problem hiding this comment.
Better, but this is now assigning a base_type to non-draconians by mistake. It also does a double lookup in the success case.
I think it should probably be something like this:
monster_type base_type = MONS_NO_MONSTER;
if (mons_is_draconian_job(mon_num))
{
// Classed draconians have fixed colours since 0.28
const auto colour_it = draconian_job_to_color.find(mon_num);
if (colour_it != draconian_job_to_colour.end())
base_type = colour_it->second;
else
// Might be better to show all possible combinations rather than
// picking one at random as this does?
base_type = random_draconian_monster_species();
}
nit: this codebase afaik uses the British spelling of colour
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.
In #4001, when you lookup information about draconian stormcaller, the color is randomly generated instead of white. This is because of lines lookup-help.cc:1061-1062. This commit attempts to fix that by assigning colors to them using a switch-case statement. Info about which color should be which is grabbed from lines mon-util.cc:2723-2728.
Should resolve #4001.