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

Change switch-case for std::pair in controls.cpp #603

Open
bufalo1973 opened this issue Jan 29, 2021 · 1 comment
Open

Change switch-case for std::pair in controls.cpp #603

bufalo1973 opened this issue Jan 29, 2021 · 1 comment

Comments

@bufalo1973
Copy link

I think it would be better to change the switch in void Controls_ActionToStr(char buff[128], ACTIONS act) to something like:

#include

std::pair<ACTIONS, std::string> ActionStr[] = {
std::make_pair(ACT_UP, "ACT_UP"),
std::make_pair(ACT_DOWN, "ACT_DOWN"),
std::make_pair(ACT_LEFT, "ACT_LEFT"),
std::make_pair(ACT_RIGHT, "ACT_RIGHT"),
std::make_pair(ACT_ACTION, "ACT_ACTION"),
std::make_pair(ACT_JUMP, "ACT_JUMP"),
std::make_pair(ACT_ROLL, "ACT_ROLL"),
std::make_pair(ACT_DRAWWEAPON, "ACT_DRAWWEAPON"),
std::make_pair(ACT_LOOK, "ACT_LOOK"),
std::make_pair(ACT_WALK, "ACT_WALK"),
std::make_pair(ACT_SPRINT, "ACT_SPRINT"),
std::make_pair(ACT_CROUCH, "ACT_CROUCH"),
std::make_pair(ACT_STEPLEFT, "ACT_STEPLEFT"),
std::make_pair(ACT_STEPRIGHT, "ACT_STEPRIGHT"),
std::make_pair(ACT_LOOKUP, "ACT_LOOKUP"),
std::make_pair(ACT_LOOKDOWN, "ACT_LOOKDOWN"),
std::make_pair(ACT_LOOKLEFT, "ACT_LOOKLEFT"),
std::make_pair(ACT_LOOKRIGHT, "ACT_LOOKRIGHT"),
std::make_pair(ACT_NEXTWEAPON, "ACT_NEXTWEAPON"),
std::make_pair(ACT_PREVWEAPON, "ACT_PREVWEAPON"),
std::make_pair(ACT_FLARE, "ACT_FLARE"),
std::make_pair(ACT_BIGMEDI, "ACT_BIGMEDI"),
std::make_pair(ACT_SMALLMEDI, "ACT_SMALLMEDI"),
std::make_pair(ACT_WEAPON1, "ACT_WEAPON1"),
std::make_pair(ACT_WEAPON2, "ACT_WEAPON2"),
std::make_pair(ACT_WEAPON3, "ACT_WEAPON3"),
std::make_pair(ACT_WEAPON4, "ACT_WEAPON4"),
std::make_pair(ACT_WEAPON5, "ACT_WEAPON5"),
std::make_pair(ACT_WEAPON6, "ACT_WEAPON6"),
std::make_pair(ACT_WEAPON7, "ACT_WEAPON7"),
std::make_pair(ACT_WEAPON8, "ACT_WEAPON8"),
std::make_pair(ACT_WEAPON9, "ACT_WEAPON9"),
std::make_pair(ACT_WEAPON10, "ACT_WEAPON10"),
std::make_pair(ACT_BINOCULARS, "ACT_BINOCULARS"),
std::make_pair(ACT_PLS, "ACT_PLS"),
std::make_pair(ACT_PAUSE, "ACT_PAUSE"),
std::make_pair(ACT_INVENTORY, "ACT_INVENTORY"),
std::make_pair(ACT_DIARY, "ACT_DIARY"),
std::make_pair(ACT_MAP, "ACT_MAP"),
std::make_pair(ACT_LOADGAME, "ACT_LOADGAME"),
std::make_pair(ACT_SAVEGAME, "ACT_SAVEGAME"),
std::make_pair(ACT_CONSOLE, "ACT_CONSOLE"),
std::make_pair(ACT_SCREENSHOT, "ACT_SCREENSHOT"),
std::make_pair(ACT_LASTINDEX, (std::string)NULL)
};

void Controls_ActionToStr(char buff[128], ACTIONS act)
{
strncpy(buff, ActionStr[act].second.c_str(), 128);
};

It might be a little faster.

@stohrendorf
Copy link
Contributor

  1. please use code markup
  2. the std::make_pair(ACT_WEAPON9, "ACT_WEAPON9"), etc. can be replaced with a temporary macro like #define ENUM_MEMBER(name) std::make_pair(name, #name)
  3. you can't construct an std::string from a nullptr
  4. using an array means you rely upon ACTIONS being contiguous and starting from 0
  5. you also need to ensure that the ordering of the members matches exactly the enum members
  6. ... unless you meant to write a loop in Controls_ActionToStr; in this case, ACT_LASTINDEX should be the default if you don't find a matching entry in the array
  7. or use a plain std::map<ACTION, std::string> which you actually want to do here, but performance of small maps (less than ~100 elements) is far inferior to linear scans
  8. std::strncpy does not guarantee that it will create a NUL-terminated string if the source (excluding the terminator) has the same length or is larger than the destination, so you'd need to assert that the source length is less than 128

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

No branches or pull requests

2 participants