-
Notifications
You must be signed in to change notification settings - Fork 92
tweak(client): Show additional information for selected objects while observing #1479
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
tweak(client): Show additional information for selected objects while observing #1479
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect there will be a lot more changes necessary to get the full observer experience. Are there plans to expand this more?
Generals/Code/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp
Outdated
Show resolved
Hide resolved
That is true, though they are likely better off as separate changes as they are more involved and require different solutions. The selection logic especially needs looking at (e.g. player relationship comparisons still use the local player when observing a player). |
Player *player = ThePlayerList->getLocalPlayer(); | ||
Player* player = TheControlBar->getCurrentlyViewedPlayer(); | ||
if (!player) | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer to not use returns in the middle of functions, because it makes logic eventually more difficult to follow along
Can simplify to:
if (Player* player = TheControlBar->getCurrentlyViewedPlayer())
{
if (TheGlobalData->m_timeOfDay == TIME_OF_DAY_NIGHT)
marker->setIndicatorColor( player->getPlayerNightColor() );
else
marker->setIndicatorColor( player->getPlayerColor() );
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I typically try to draw the line at three levels of nesting. The end of the loc == NULL
block should really be a return
instead of an else
. I'm also wary of putting assignments within conditions, which can be useful in specific situations (i.e. reducing loop verbosity), but is otherwise generally discouraged. I'll give the method a readability refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to be discouraged to put assignments into conditions. In modern C++ this is even made explicit with if (expr; cond)
syntax. The benefit of doing that is the smaller scope for the pointer, which avoids misuse at compile time.
The confusion with returns, other than those on early outs or final return, can make the flow of the function more difficult to follow, depeneding on the overall complexity of the function. Early returns also put a strict requirement on use of RAII, because otherwise resources may be left leaking. Furthermore, any trailing critical logic may be left uncalled. Early returns are a very common source of error.
This change expands object selection information for observers. While a player is selected, the following information is now displayed for selected objects: