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

Migrate ACT_CRAFT to activity actors, fix activity actor migration, allow activity actors to specify specific progress messages #42794

Merged
merged 3 commits into from
Aug 19, 2020

Conversation

anothersimulacrum
Copy link
Member

@anothersimulacrum anothersimulacrum commented Aug 8, 2020

Summary

SUMMARY: None

Purpose of change

Migrate ACT_CRAFT to be an activity actor, fix bugs that sprung up while doing so.

Describe the solution

Fix actor migration cancelling
f683059

The check for whether or not the activity needed to be cancelled due to being migrated to an activity actor was incorrect - the function to serialize a player_activity would not omit the member if the actor was null, it would write out a null.
That is, for all activities saved from versions after activity actors were introduced, so also keep the old check for the member not existing.

After fixing this, I noticed that it still wasn't working - so check if the type is a cancel activity in player_activity::do_turn, and cancel if so.

Allow activity actors to specify progress messages
39fb139

Instead of the hardcoded map of id->message, let actors specify whatever message they want for the progress message. Utilize this for the two existing actors using it.

Migrate ACT_CRAFT to actors
feccd57

Change some pointers to references, refactor out a function or two for cleanliness, but mostly just moving code around.

Testing

Crafting from hands, examine menu, and crafting menu to completion works correctly. Crafting repeatedly from the examine menu works correctly.
Loading a savegame with an existing craft action in progress cancels correctly.

Additional context

Yak-shaving for future work involving recipes.

@anothersimulacrum anothersimulacrum added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style <Bugfix> This is a fix for a bug (or closes open issue) labels Aug 8, 2020
@anothersimulacrum anothersimulacrum force-pushed the actriz branch 3 times, most recently from feccd57 to bb7d3c5 Compare August 8, 2020 14:39
The check for whether or not the activity needed to be cancelled due to
being migrated to an activity actor was incorrect - the function to
serialize a player_activity would not omit the member if the actor was
null, it would write out a null.
That is, for all activities saved from versions after activity actors
were introduced, so also keep the old check for the member not existing.

After fixing this, I noticed that it still wasn't working - so check if
the type is a cancel activity in player_activity::do_turn, and cancel if
so.
Instead of the hardcoded map of id->message, let actors specify whatever
message they want for the progress message. Utilize this for the two
existing actors using it.
Change some pointers to references, refactor out a function or two for
cleanliness, but mostly just moving code around.
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` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants