Opened 8 years ago
Closed 8 years ago
#3991 closed enhancement (fixed)
[PATCH] Make SimContext const again
Reported by: | elexis | Owned by: | |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 21 |
Component: | Core engine | Keywords: | patch |
Cc: | Patch: |
Description
r17735 removed the const
keyword from the Simulatons CSimContext
so that GetCurrentDisplayedPlayer
could be changed for observers. However having the const keyword means that attempts to change the object which potentially could cause harm are detected at compiletime, so removing it is not ideal.
The attached patch adds back the const keyword and stores the viewed player in g_Game
instead, thus also removes the m_CurrentDisplayedPlayer
from r8160.
Another potential optimization would be to also remove GetCurrentDisplayedPlayer()
from the SimContext and directly access g_Game
. If doing so, one has to be careful not to introduce OOS issues.
Attachments (1)
Change History (8)
by , 8 years ago
Attachment: | t3991_const_simcontext_v1.patch added |
---|
comment:1 by , 8 years ago
follow-up: 4 comment:2 by , 8 years ago
Keywords: | review removed |
---|
Thanks leper for being persistent on this one and giving an ack on the patch. I didn't like to remove the const at the time either, but didn't see the obvious solution to store the var in a different place :)
Not closing the ticket until it is certain whether GetCurrentDisplayedPlayer
can be nuked as well.
comment:4 by , 8 years ago
Replying to elexis:
Not closing the ticket until it is certain whether
GetCurrentDisplayedPlayer
can be nuked as well.
Was this tested? It should be committed quickly, so OOS can be found during the testing phase of the release.
comment:5 by , 8 years ago
Looking at the code:
/** * Contains pointers to various 'global' objects that are needed by the simulation code, * to allow easy access without using real (evil) global variables. */ class CSimContext .... /** * Returns the player ID that the current display is being rendered for. * Currently relies on g_Game being initialised (evil globals...) */
the idea of having the SimContext must have been to avoid using globals. Otherwise the simulation could just refer to the globals and nuke the SimContext altogether. So r18201 shouldn't have passed the review either then and the simcontext will have to save the playerID in some member variable, or a member of a member. Perhaps it's possible to do r17735 with some C++ trick to avoid the const change.
comment:6 by , 8 years ago
I don't understand your last comment. The only problem with the code right now is that g_Game
is a global variable, but r18201 is perfectly fine.
comment:7 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Created a ticket #4211 for removing references to globals, which doesn't have to be solved this alpha and is a larger task.
In 18201: