-
Notifications
You must be signed in to change notification settings - Fork 71
Split AI files, utility calculations for CB, allow addition of CB midwar for AI #1968
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
Conversation
@@ -1047,6 +1047,17 @@ void write_save_file(sys::state& state, save_type type, std::string const& name) | |||
|
|||
state.save_list_updated.store(true, std::memory_order::release); // update for ui | |||
|
|||
// log count of pressed wargoals | |||
{ |
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.
I imagine that you didn't mean to leave this in (or you forgot to hide it behind a condition.
src/ai/ai_war.cpp
Outdated
dcon::nation_id owner, | ||
dcon::state_instance_id target | ||
) { | ||
auto victim = state.world.state_instance_get_nation_from_state_ownership(target); |
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.
You don't appear to use this variable. Did you mean to use it instead of owner at some point? Owner is a bit confusing as a parameter here because I assume you mean that it is the nation that the utility is being calculated from the point of view of
src/ai/ai_war.cpp
Outdated
|
||
void prepare_and_sort_list_of_desired_states( | ||
sys::state& state, | ||
std::vector<ai::weighted_state_instance>& result, |
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.
assuming that you don't intend to call this function repeatedly with the same vector in order to append results to a single vector, this function (and the one below) could be written to return the std::vector result without any loss of efficiency because of what is known as the return value optimization, aka copy elision. Basically, if a function is always returning a given object (i.e. you don't construct two different vectors in the function and then conditionally return one or the other), then the compiler is allowed to implement a function returning an object as if it took a pointer to uninitialized memory for that object, in which it constructs the object that it will ultimately return, meaning that no copying has to be done, and so it can be just as efficient as what you have written here, but with a clearer expression of what is the input and the outputs. You can read the notes here https://en.cppreference.com/w/cpp/language/return.html#Notes if you are curious about tracking down the details of when copy elision can be expected.
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.
The function below is used repeatedly, but this one could be made into a function which returns the vector, thanks. Actually these functions were already like this, I just edited them to use utility.
} | ||
} | ||
|
||
bool will_accept_crisis_peace_offer(sys::state& state, dcon::nation_id to, bool is_concession, bool missing_wg) { |
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.
If the logic for these functions has changed, then there may be some ui elements that need to be changed as well (if the tooltip on why the ai will/won't accept gives details)
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.
I haven't changed the functions which influence AI agreement, my changes are focused on changing behavior of selecting a wargoal in different contexts
} | ||
|
||
// Make sure no country higher than originator on rank is justifying on that target | ||
for(auto n : state.nations_by_rank) { |
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.
IIRC, nations_by_rank is larger than the number of actual nations and is filled up with invalid ids as padding, so you should probably add an if(!n) break; to exit the loop early when you hit that padding.
Before introduction of local administrative efficiency taking random states in the middle of nowhere was a fine decision. But currently it's more optimal to avoid exclaves as you can't collect taxes/tariffs there without establishment of local administration.
In this PR I do the following: