Skip to content
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

Clean up aim_activity_actor #42264

Merged
merged 1 commit into from
Jul 20, 2020
Merged

Clean up aim_activity_actor #42264

merged 1 commit into from
Jul 20, 2020

Conversation

olanti-p
Copy link
Contributor

@olanti-p olanti-p commented Jul 19, 2020

Summary

SUMMARY: None

Purpose of change

Simplifies aim_activity_actor as a follow-up after #40684
Fixes #42250

The crash happened due to the following coincidence:

  • aiming was finished with player having exactly 0 moves left
  • the woodbow had 1 turn left before expiring

aim_activity_actor::do_turn(...) method requires an extra call for marking the activity as finished, so in a case when aiming was finished with player having exactly 0 moves left, that extra call was delayed by 1 turn, and so aim_activity_actor::finish(...) was also delayed by 1 turn.

That 1-turn delay was enough for the woodbow to disappear.

But, finish(...) method assumed that the weapon checks done back in do_turn(...) were still relevant, so it simply dereferenced the weapon pointer, which have turned into a null pointer by that time, without validation.

Describe the solution

Removed the need for an extra call by inserting act.moves_left = 0 in do_turn(...) wherever necessary.
Added a nullptr check in finish(...) for robustness in the future.
Also, noticed that my old implementation used shared pointers instead of (de-)serializable and more convenient cata::optional, so changed that.
Removed an enum that was basically a leftover from refactoring.

Describe alternatives you've considered

Just slapping a nullptr check, but that's treating symptoms, not the cause.

Testing

Set (via debugger) avatar's moves and the woodbow's timer to expire as described. The game crashed before, but after this fix the bow allows the last shot before disappearing.

@anothersimulacrum anothersimulacrum added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` labels Jul 20, 2020
@kevingranade kevingranade merged commit 3312299 into CleverRaven:master Jul 20, 2020
@olanti-p olanti-p deleted the clean-aim-act-act branch July 20, 2020 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segfault firing woodbow at zombies
3 participants