Skip to content

inserted missing implementation for some GameAction subclasses#13616

Closed
csunday95 wants to merge 57 commits into
OpenRCT2:developfrom
csunday95:fix-13615-incomplete-game-actions
Closed

inserted missing implementation for some GameAction subclasses#13616
csunday95 wants to merge 57 commits into
OpenRCT2:developfrom
csunday95:fix-13615-incomplete-game-actions

Conversation

@csunday95

Copy link
Copy Markdown
Contributor

Creating pull request for fixing issue #13615

So far only has SetStaffNameAction implementation, but am planning to browse through the actions and see if any others need the same additions

@Gymnasiast Gymnasiast requested a review from IntelOrca December 21, 2020 13:01
duncanspumpkin and others added 2 commits December 21, 2020 13:25
This can be used in conjuction with OpenRCT2#13593 to change the placement of vehicles on a track

void StaffSetNameAction::AcceptParameters(GameActionParameterVisitor& visitor)
{
visitor.Visit("spriteIndex", _spriteIndex);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just id would be better than spriteIndex. We no longer refer to entities as sprites.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In GuestSetNameAction we used peep

@csunday95 csunday95 Dec 21, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just selected spriteIndex to line up with the member name, would it be preferred to have a more human readable name like "peep" or "peepId" (or perhaps "staffId") for the outward facing API?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think peep would be a best. There are at least 2 other actions that use that.

Gymnasiast and others added 5 commits December 21, 2020 18:40
* fix for bug OpenRCT2#13611; handled too few sig figs fixed point case

* added unit test cases to cover modified fixed point code

* removed blank line to satistfy clang-format

* sorted includes in formatting tests for clang-format

* removed redundant static_cast

 - already a char literal and assigning to char[] so code
 is functionally equivalent
uint32_t flags = GetFlags();

if (!(GetFlags() & (GAME_COMMAND_FLAG_ALLOW_DURING_PAUSED | GAME_COMMAND_FLAG_GHOST)))
if (!(flags & (GAME_COMMAND_FLAG_ALLOW_DURING_PAUSED | GAME_COMMAND_FLAG_GHOST)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't make unrelated changes in this PR save it for a different PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait... how did this end up here? dang I think I must have been working on the wrong local branch when I was investigating something. I'll revert this on my next commit to this PR

…g enum (OpenRCT2#13103)

Merge MISC_COMMAND and GAME_COMMAND enums

Cleanup
@csunday95

Copy link
Copy Markdown
Contributor Author

ah shoot I seem to have messed up this PR while trying to rebase something else; not sure what to do in this situation since I committed on top of the rebase and pushed. Might just be best to close this PR and redo it from scratch? redoing these changes is pretty trivial with PR this as a reference

@duncanspumpkin

Copy link
Copy Markdown
Contributor

Just do a rebase of develop. It will remove all of the merge related commits that you have added and keep only your changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants