Page MenuHomeWildfire Games

Add path list to Atlas
ClosedPublic

Authored by vladislavbelov on Apr 17 2017, 4:19 PM.

Details

Reviewers
elexis
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP19427: List, add and delete cinematic paths in Atlas.
Summary

Adds a path list, buttons to add/delete paths.

Test Plan
  1. Open atlas
  2. Try to add/delete paths

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 17 2017, 4:19 PM
Vulcan added a subscriber: Vulcan.Apr 17 2017, 5:04 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/797/ for more details.

elexis accepted this revision.Apr 18 2017, 5:19 AM

I accept this under the condition that this is an incremental step and that the controls to add path nodes is coming in the following days / weeks, because users will be confused if they see controls to add a path but not nodes (whereas with current svn, we can only trigger the visibility, which might be more expected if the features are not complete).

source/simulation2/components/ICmpCinemaManager.cpp
1 ↗(On Diff #1303)

not this time!

source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Cinema/Cinema.cpp
66 ↗(On Diff #1303)

Border(wxALL, 5)? But I'm grasping at straws

107 ↗(On Diff #1303)

\n after return; imo

127 ↗(On Diff #1303)

We avoid the auto type as far as possible, so that we immediately become aware of the type used

source/tools/atlas/GameInterface/Handlers/CinemaHandler.cpp
190 ↗(On Diff #1303)

There should be atlas controls to select these values sometime.

194 ↗(On Diff #1303)

Wasn't entirely convinced of adding the first node at the camera position, but perhaps it can be expected / helpful. I'll stay reserved and we should reconsider this initial node once we have more controls.

197 ↗(On Diff #1303)

As discussed in IRC, having hardcoded numbers raises the question where they come from. As you said it's the average height of a unit and it's ok to remove the number as well, going for that.

This revision is now accepted and ready to land.Apr 18 2017, 5:19 AM
This revision was automatically updated to reflect the committed changes.