Move CCinemaPath to simulation2/helpers
ClosedPublic
Actions

Authored by vladislavbelov on Apr 13 2017, 1:54 AM.

Details

Reviewers

elexis
wraitii

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP19414: Move CinemaPath class to the simulation helper directory, because it contains…

Trac Tickets

#3814

Summary

Since the CCinemaPath is the simulation part, it should be in simulation directory, but there isn't any special for that, so @wraitii suggested to use simulation2/helpers/.

Also CCinemaPath has some real time data, but it's only to cache some data, it's not serializing and never used in simulation.

Test Plan

Run the game
Open any map with paths
Test that all work correctly

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vladislavbelov created this revision.Apr 13 2017, 1:54 AM

vladislavbelov added a parent revision: D317: Add type conversation for CCinemaPath for TriggerScripts.

Build has FAILED

Link to build: http://jw:8080/job/phabricator/753/
See console output for more information: http://jw:8080/job/phabricator/753/console

Harbormaster failed remote builds in B1102: Diff 1225!Apr 13 2017, 1:55 AM

Also CCinemaPath has some real time data, but it's only to cache some data, it's not serializing and never used in simulation.

Wouldn't it be preferable to have one class with the simulation data only and one graphics class that is initialized from the simulation data and stores the non-simulation data (for example floats for interpolation)?

Wouldn't it be preferable to have one class with the simulation data only and one graphics class that is initialized from the simulation data and stores the non-simulation data (for example floats for interpolation)?

As I said yesterday on IRC, I think it depends how much extra "graphics-only" data we need. If it's small, it it probably better to keep it all in one place (the simulation). However, if it's a lot of data, it's probably going to end up cleaner to split it properly.
For example, Rallypoints are only in the simulation, including the graphics stuff - but that's a lot of extraneous code that maybe should be relegated to graphics/
OverlayRenderer also is graphics-only code but it's small enough that it can be kept in the sim. Likewise with CmpDecay.
And OTOH, we have for example the water renderer where the code is mostly graphical so splitting it makes more sense.

@leper WWLD :<

Split it if there is purely graphics only data.

Vladislav and me discussed the diff today a bit on IRC and the plan is to remove the non-simulation data altogether from the CinemaPath instead of moving it and to move the graphical functions.
So it is acceptable to me to move the class that predominantly contains simulation data and is already serialized to the simulation helper directory as suggested by wraitii.

A condition for the acceptance is that that the number of non-simulation inclusions are reduced as far as possible, that we throw out the following unused ones.

#include "CinemaManager.h"
#include "GameView.h"
#include "gui/CGUI.h"
#include "gui/GUIManager.h"
#include "gui/IGUIObject.h"
#include "lib/ogl.h"
#include "maths/Vector4D.h"
#include "renderer/Renderer.h"
#include "ps/Game.h"