Skip to content

WIP: Closes #12419: Combine MISC_COMMAND and GAME_COMMAND into strong enum#13103

Merged
tupaschoal merged 1 commit into
OpenRCT2:developfrom
n7st:#12419-misc_command
Dec 22, 2020
Merged

WIP: Closes #12419: Combine MISC_COMMAND and GAME_COMMAND into strong enum#13103
tupaschoal merged 1 commit into
OpenRCT2:developfrom
n7st:#12419-misc_command

Conversation

@n7st

@n7st n7st commented Oct 5, 2020

Copy link
Copy Markdown
Contributor

This pull request reconciles MISC_COMMAND and GAME_COMMAND, refactoring them both to use strong enums. I'm currently testing the changes.

Thanks!

Comment thread src/openrct2/network/NetworkAction.h Outdated
Comment thread src/openrct2/network/NetworkBase.cpp Outdated
Comment thread src/openrct2/network/NetworkAction.cpp Outdated
@n7st n7st changed the title Closes #12419: Refactor MISC_COMMAND to use strong enum WIP: Closes #12419: Refactor MISC_COMMAND to use strong enum Oct 6, 2020

@n7st n7st left a comment

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.

There are a few unknowns here still. I'm working through testing.

Comment thread src/openrct2/Game.h
Comment thread src/openrct2/actions/GameAction.h
Comment thread src/openrct2/network/NetworkBase.cpp
@n7st n7st changed the title WIP: Closes #12419: Refactor MISC_COMMAND to use strong enum WIP: Closes #12419: Combine MISC_COMMAND and GAME_COMMAND, refactor to use strong enum Oct 6, 2020
@n7st n7st changed the title WIP: Closes #12419: Combine MISC_COMMAND and GAME_COMMAND, refactor to use strong enum WIP: Closes #12419: Combine MISC_COMMAND and GAME_COMMAND into strong enum Oct 6, 2020
@ZehMatt

ZehMatt commented Oct 6, 2020

Copy link
Copy Markdown
Contributor

I think the changes should be reverted back to using a specific enum for the time being, ideally we refactor the network packets later to have individual packets rather than one packet with a sub command.

@ZehMatt

ZehMatt commented Oct 9, 2020

Copy link
Copy Markdown
Contributor

The PR looks alright, the CI is currently failing due to the commit messages? I have no objections to merge this as is, the special game commands needs fixing after this PR.

@tupaschoal

Copy link
Copy Markdown
Member

Please rebase to drop the merge commit and fold the fourth one into one of the others, this will solve the lint commit problem.

Also, one of the debug builds failed with:

`GameActions::ExecuteInternal(GameAction const*, bool)': 

995/__w/OpenRCT2/OpenRCT2/bin/../src/openrct2/actions/GameAction.cpp:488: undefined reference to `network_set_player_last_action(unsigned int, GameCommand)'

Which is a bit suspicious

@tupaschoal

Copy link
Copy Markdown
Member

ugh, looks like #13184 introduced a boatload of merge conflicts, sorry. Can you please rebase again? Or maybe @ZehMatt wants to be kind and solve them :D

@n7st

n7st commented Oct 15, 2020

Copy link
Copy Markdown
Contributor Author

ugh, looks like #13184 introduced a boatload of merge conflicts, sorry. Can you please rebase again? Or maybe @ZehMatt wants to be kind and solve them :D

No worries, I'll take a look later on today. It's all good practice!

Merge MISC_COMMAND and GAME_COMMAND enums

Cleanup
@tupaschoal tupaschoal merged commit 4e991be into OpenRCT2:develop Dec 22, 2020
@tupaschoal tupaschoal added this to the v0.3.3 milestone Dec 22, 2020
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.

4 participants