Feature: [GS] Goal destination can be updated #10817
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return (type == GT_NONE && destination == 0) || | ||
| (type == GT_TILE && ScriptMap::IsValidTile(destination)) || | ||
| (type == GT_INDUSTRY && ScriptIndustry::IsValidIndustry(destination)) || | ||
| (type == GT_TOWN && ScriptTown::IsValidTown(destination)) || | ||
| (type == GT_COMPANY && ScriptCompany::ResolveCompanyID((ScriptCompany::CompanyID)destination) != ScriptCompany::COMPANY_INVALID) || | ||
| (type == GT_STORY_PAGE && story_page != nullptr && (c == INVALID_COMPANY ? story_page->company == INVALID_COMPANY : story_page->company == INVALID_COMPANY || story_page->company == c))); | ||
| (type == GT_STORY_PAGE && story_page != nullptr && (c == INVALID_COMPANY ? story_page->company == INVALID_COMPANY : story_page->company == INVALID_COMPANY || story_page->company == c)); |
Check notice
Code scanning / CodeQL
Complex condition Note
There was a problem hiding this comment.
Could this use the new IsValidGoalDestination function?
There was a problem hiding this comment.
It's something I considered. The original author had this code written twice like this. Without understanding the original intention I followed suit.
If I had to guess, the intention seems to be to use functions like ScriptMap::IsValidTile instead of ::IsValidTile, but these functions in ScriptMap mostly just call the global functions anyway. Maybe the intention is to try and make sure the checks obey the logic exposed to the script api.
If you agree this isn't a concern I can replace this with a call to Goal::IsValidGoalDestination
Motivation / Problem
See #10503
The destination of an existing goal object cannot be updated.
Description
I have added the relevant method to the GS.
I refactored the way validity of the goal destination is checked since it's now checked both on creation of goals and on updating the destination.
E.g.
GSGoal.SetDestination(gid, GSGoal.GT_TOWN, 2)Limitations
The company of an existing goal object still cannot be updated. I'm not sure of the full implications of doing this.
I'm also not sure what
CMD_STR_CTRLdoes regarding the other goal update methods (e.g. set completed), but since they use it, I used it too.Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.