• Unrelated: When someone rejoins while a path is played, there is no OOS, but the path should be played for the rejoined client too, at the same state like for the other clients
• Unrelated: It doesn't consider gamespeed. If it is 20x, the sim time advances by 20x while the path is played at regular speed and aborts after 2 seconds.
• Unrelated: The cinematic path doesn't play if the window doesn't have focus. This is good for singleplayer but probably bad for multiplayer.

(A combination of one of these points means that in MP, the paths aren't played exactly at the same time, which should likely be the case.)

Things wrong in this patch:

• The code moves the disabling of unit silhouettes in the renderer to the simulation component, but should remain in the graphics class.

• Three m_Paused:

The graphics m_Paused is just a striaght copy from the g_Game one and we can just check for the latter.
The simulation m_Paused is unused / never set to true and shouldn't be used in the future either. Explicit pausing will most likely never be part of the simulation (as players should be able add a pause even if other players paused already) and the sim must treat implicit pauses (lag) equally well.
Having three different variables for that is bad as it means they could have different states if not managed correctly and it means that the reader has to figure out the differences and prove the correctness too.
https://en.wikipedia.org/wiki/Single_source_of_truth

• m_Enabled graphics copy

m_Enabled in the simulation is true if the path is currently playing or paused.
m_Enabled in the graphics class is solely used as a cache to prevent calling sn->SetSetting("hidden", L"true"); on each call.
However we are much better off with checking for GUI<bool>::GetSetting(sn, "hidden", hidden); instead of adding new member variable that is a copy of the simulation variable.

• More readability if logically independent things would be moved to helper functions like UpdateSessionVisibility, UpdateSilhouettesVisibility and DrawPaths instead of combining it in one function.

The reader can first read the structure and then only read the code if he really wants to. Also allows early returns instead of big if-scopes.

Things wrong in rP17594 which this patch could have addressed:

• Hotkey TODO:
  
  Not changed by the patch but still in the hunks: // TODO: implement skip with some empty code which also seems wrong. That TODO shouldn't have been in the code in the first place. trac is the place for a wish list. The empty code shell shouldn't have been added. Also the code shell is probably wrong because it must be a simulation command, not a GUI check, so it probably must go to misc.xml. TODO features must be designed in the proper place with dev feedback before implemented partially.

• Searching for the uses of the removed member variables above revealed CameraCtrlHandlers.cpp should check for IsEnabled, not IsPlaying because we shouldn't be able to move the camera while a cinematic path is paused too

• GetPathsDrawing and SetPathsDrawing are unused and should not have been introduced, they just confuse the reader. Introduce things only when they are needed or when a de is guaranteed to commit the user of that code right afterwards. Not removing it now because you said you will use it soon.

*Things which will remain wrong after this diff*

• OOS on rejoin while a path is played, because the path won't be started for a rejoining client if it had started before (should be implemented) and thus the reveal map flag will not be reset
• Cinematic camera path animation is not synchronized but only depends on renderer, thus ignores gamespeed, network and renderer lag. We must guarantee that all players finished the path simultaneously and that if a map/path author wants to show a given entity at a given time, that the path actually follows that determination.