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

move plmove, ramp_move, autoattack to namespace #30794

Merged
merged 5 commits into from
May 24, 2019

Conversation

KorGgenT
Copy link
Member

Summary

SUMMARY: Infrastructure "Move game::plmove(), game::ramp_move() and game::autoattack() to avatar_action namespace"

Purpose of change

There's a whoe bunch of functions that don't really live in the right place: these are examples of functions that don't really need to be owned by game.

I chose the name avatar_action because there are a bunch of other functions in game that could also go here, and all of these things have an effect on the player, but may not necessarily want to be owned by avatar either.

Describe alternatives you've considered

putting these functions into the avatar class. i i decided against it because there's a lot of stuff that will go into that class in time anyway, and keeping it to a reasonable size would be a reasonable course of action while it's still in the process of being refactored.

Additional context

I needed to move a couple of things from private to public in game, though some of those will be moved out of game anyway eventually.

@KorGgenT KorGgenT added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style labels May 23, 2019
@mlangsdorf
Copy link
Contributor

I think I'd rather see these written as avatar_action::move() than defining them inside the namespace's scope, even though they're functionally equivalent. Specifying the namespace inside the definition makes it easier to see when grep'ing.

@KorGgenT
Copy link
Member Author

that's reasonable.

@ZhilkinSerg ZhilkinSerg merged commit 917a7dd into CleverRaven:master May 24, 2019
@ZhilkinSerg ZhilkinSerg removed their assignment May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[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.

None yet

3 participants