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
Corrected world level incursion search criteria #1713
Conversation
plugins/embark-assistant/matcher.cpp
Outdated
@@ -1173,7 +1173,7 @@ namespace embark_assist { | |||
else if (k < start_y + finder->y_dim - 1) { // We've already covered the SE tile's SE corner, with its complications. | |||
process_embark_incursion_mid_level_tile | |||
(embark_assist::survey::translate_corner(survey_results, | |||
8, | |||
embark_assist::defs::directions::Southwest, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8 is Southeast, not Southwest - was this changed intentionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that's a bug (the danger of improving stuff that isn't broken). I'll fix it. And thanks for detecting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the other changes looked good, by the way (forgot to mention that)
@@ -314,7 +314,19 @@ command_result embark_assistant(color_ostream &out, std::vector <std::string> & | |||
embark_assist::main::state->survey_results[i][k].survey_completed = false; | |||
embark_assist::main::state->survey_results[i][k].neighboring_clay = false; | |||
embark_assist::main::state->survey_results[i][k].neighboring_sand = false; | |||
embark_assist::main::state->survey_results[i][k].neighboring_aquifer = embark_assist::defs::Clear_Aquifer_Bits; | |||
for (uint8_t l = 0; l <= ENUM_LAST_ITEM(biome_type); l++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also a FOR_ENUM_ITEMS macro in DataDefs.h, in case you were unaware - you're perfectly welcome not to use it, though (for stylistic reasons or otherwise - it does yield values of the enum type instead of the base integer type, although I think either would work here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I didn't know that.
@@ -50,6 +50,7 @@ changelog.txt uses a syntax similar to RST, with a few special sequences: | |||
- `buildingplan`: new global settings for whether generic building materials should match blocks, boulders, logs, and/or bars. defaults are everything but bars. | |||
- `quickfort`: The Dreamfort sample blueprints now have complete walkthroughs for each fort level and importable orders that automate basic fort stock management | |||
- `quickfort`: More blueprints have been added to the blueprints library: several bedroom layouts, the Saracen Crypts, and the complete fortress example from Python Quickfort: TheQuickFortress | |||
- `embark-assistant`: split the lair types displayed on the local map into mound, burrow, and lair |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating this - are the other changes in this PR essentially a continuation of #1704?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's a continuation of that PR, so the line in the changelog covers it. The magic number->named number change is too internal and marginal to get a description, as it doesn't modify the functionality (apart from introduced bugs...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thanks for confirming
In addition to correcting the top level filtering, a few other things were made: