Page MenuHomeWildfire Games

Waypoint Flags
Needs ReviewPublic

Authored by Stan on May 26 2017, 10:02 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#657
Summary

The Waypoint flags should have the following properties:

Factional: I've made 6 waypoint flags, one for each of the game's factions. I've put them all in one actor, with 6 variations, one for each faction. Let me know if this is adequate or if 6 separate actors are needed (very easy for me to make).
Implemented as of [10704]. 

Visibility: Waypoint flags should be visible in FOW (Fog of War) foremost; secondarily could possibly be visible in SOD (Shroud of Darkness).
See #908. 

Extended to Units: Waypoint flags should show up for unit movement as well, not just for building rally points. Tasking a unit to move somewhere should show a Waypoint flag at that spot. Selecting a moving unit should show a flag at where he is tasked to move. Tasking a unit to different waypoints should show a flag at each waypoint. These would only show up when the unit is selected and if the unit is one under the control of the player.
(Related to #1053)
Test Plan

Run the tests. I plan to write some more tests if that is relevant/useful/not a review timewaste.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 2550
Build 4315: Vulcan BuildJenkins

Event Timeline

Stan created this revision.May 26 2017, 10:02 PM
Stan updated this revision to Diff 2229.May 26 2017, 11:14 PM

Remove autoformat from vscode.

Stan updated this revision to Diff 2237.May 27 2017, 1:03 PM

Remove code duplication in gui interface.

Stan updated this revision to Diff 2260.May 27 2017, 11:30 PM

Move code to a parent class and inherit from it.

Stan updated this revision to Diff 2262.May 28 2017, 12:47 AM

Fix a crash when patrolling. std::dequeue.size() return weird values sometimes.

I think we shouldn't add renderers to the simulation, we've already discussed with @wraitii about this. And it's better to have renderers in a real time part. The simulation should have only a simulation data.

Also, do we really need a deque? Couldn't we use list/vector? Just wondering about performance/how often we calls different methods.

Stan updated this revision to Diff 2283.May 28 2017, 6:57 PM

Rebase it for lasts commits, reorder the function a bit

Stan set the repository for this revision to rP 0 A.D. Public Repository.May 28 2017, 6:59 PM
Stan added subscribers: Vulcan, Restricted Owners Package.
Stan updated this revision to Diff 2326.May 31 2017, 1:29 AM

Further code cleaning, move functions around to avoid duplication, and replace std::deque by vectors fix some bugs that would throw warnings.

Still some issues with formations, but I think I found a way to fix that.
Still have to figure why flags don't move out of world but just slide
Make interpolation work
Still have to find out why the lines don't disappear when deselecting.

Stan updated this revision to Diff 2793.Jul 2 2017, 7:55 PM
Stan set the repository for this revision to rP 0 A.D. Public Repository.

Make flags disappear when unselected, and fix display bug

Vulcan added a comment.Jul 2 2017, 8:43 PM

Build has FAILED

Updating workspaces.
Build (release)...
In file included from ../../../source/simulation2/components/CCmpItineraryPointRenderer.cpp:19:0:
../../../source/simulation2/components/CCmpItineraryPointRenderer.h:70:7: warning: ‘class CCmpItineraryPointRenderer’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpItineraryPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpRallyPointRenderer.h:21:0,
                 from ../../../source/simulation2/components/CCmpRallyPointRenderer.cpp:19:
../../../source/simulation2/components/CCmpItineraryPointRenderer.h:70:7: warning: ‘class CCmpItineraryPointRenderer’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpItineraryPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpRallyPointRenderer.cpp:19:0:
../../../source/simulation2/components/CCmpRallyPointRenderer.h:46:7: warning: base class ‘class CCmpItineraryPointRenderer’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpRallyPointRenderer : public CCmpItineraryPointRenderer, public ICmpRallyPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpWayPointRenderer.h:21:0,
                 from ../../../source/simulation2/components/CCmpWayPointRenderer.cpp:19:
../../../source/simulation2/components/CCmpItineraryPointRenderer.h:70:7: warning: ‘class CCmpItineraryPointRenderer’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpItineraryPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpWayPointRenderer.cpp:19:0:
../../../source/simulation2/components/CCmpWayPointRenderer.h:46:7: warning: base class ‘class CCmpItineraryPointRenderer’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpWayPointRenderer : public CCmpItineraryPointRenderer, public ICmpWayPointRenderer
       ^
Build (debug)...
In file included from ../../../source/simulation2/components/CCmpItineraryPointRenderer.cpp:19:0:
../../../source/simulation2/components/CCmpItineraryPointRenderer.h:70:7: warning: ‘class CCmpItineraryPointRenderer’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpItineraryPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpRallyPointRenderer.h:21:0,
                 from ../../../source/simulation2/components/CCmpRallyPointRenderer.cpp:19:
../../../source/simulation2/components/CCmpItineraryPointRenderer.h:70:7: warning: ‘class CCmpItineraryPointRenderer’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpItineraryPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpRallyPointRenderer.cpp:19:0:
../../../source/simulation2/components/CCmpRallyPointRenderer.h:46:7: warning: base class ‘class CCmpItineraryPointRenderer’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpRallyPointRenderer : public CCmpItineraryPointRenderer, public ICmpRallyPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpWayPointRenderer.h:21:0,
                 from ../../../source/simulation2/components/CCmpWayPointRenderer.cpp:19:
../../../source/simulation2/components/CCmpItineraryPointRenderer.h:70:7: warning: ‘class CCmpItineraryPointRenderer’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpItineraryPointRenderer
       ^
In file included from ../../../source/simulation2/components/CCmpWayPointRenderer.cpp:19:0:
../../../source/simulation2/components/CCmpWayPointRenderer.h:46:7: warning: base class ‘class CCmpItineraryPointRenderer’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
 class CCmpWayPointRenderer : public CCmpItineraryPointRenderer, public ICmpWayPointRenderer
       ^
Running release tests...
Running cxxtest tests (306 tests).ERROR: JavaScript error: simulation/components/tests/test_WayPoint.js line 4
ReferenceError: ent is not defined
  @simulation/components/tests/test_WayPoint.js:4:5

In TestComponentScripts::test_scripts:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_WayPoint.js"
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
................................................................................................................................................................................................................................................................................................................
Failed 1 and Skipped 0 of 306 tests
Success rate: 99%

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

Stan updated this revision to Diff 2795.Jul 2 2017, 9:59 PM

Try to fix compilation warnings, fix tests.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Imarok added a subscriber: Imarok.EditedJul 3 2017, 4:59 PM

Bug: Select unit A, queue some move orders with shift, select unit B queue some move orders with shift. Now select unit A again: you will see no rally points.
Edit: building rally point lines seem to be black with this patch.

Stan added a comment.Jul 3 2017, 5:12 PM

Yeah I'm on it. I couldn't figure out why that happens. For now I had it displayed all the time so it didn't occur.

Stan updated this revision to Diff 2803.Jul 3 2017, 11:37 PM

Fix display bug, the garison points in the js component were not set. Fixed it by setting them in commands.js

Still buggy
  • Patrol : flags are not set correctly.

To be defined:

What actions should trigger the flag.
What bugs I may have introduced.

Build has FAILED

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests).ERROR: JavaScript error: simulation/components/tests/test_WayPoint.js line 8
TypeError: cmpWayPoint.AddPosition is not a function
  @simulation/components/tests/test_WayPoint.js:8:1

In TestComponentScripts::test_scripts:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_WayPoint.js"
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
................................................................................................................................................................................................................................................................................................................
Failed 1 and Skipped 0 of 306 tests
Success rate: 99%

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

Stan updated this revision to Diff 2804.Jul 4 2017, 8:29 AM

I'll investigate the black line later should fix the tests

Vulcan added a comment.Jul 4 2017, 9:19 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Stan updated this revision to Diff 2820.Jul 5 2017, 11:05 PM

Switched to git, fix bugs with rally point, such as color.
Remove a useless CcmpItineray:: prefix

Build has FAILED

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

Vulcan added a comment.Jul 7 2017, 1:35 AM

Build has FAILED

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

Vulcan added a comment.Jul 7 2017, 2:22 AM

Build has FAILED

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

Stan added a subscriber: Itms.Jul 7 2017, 8:05 AM

@Itms Can you tell me why it fails ?

Itms added a comment.Jul 7 2017, 8:33 AM
In D557#28423, @Stan wrote:

@Itms Can you tell me why it fails ?

The patch doesn't apply cleanly. A hunk fails in UnitAI.js.

Stan updated this revision to Diff 2851.Jul 7 2017, 6:18 PM

Rebase unitai

Vulcan added a comment.Jul 7 2017, 7:18 PM

Build has FAILED

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

Vulcan added a comment.Jul 7 2017, 8:15 PM

Build has FAILED

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

Stan updated this revision to Diff 2863.Jul 7 2017, 11:36 PM

Try another time with full pull.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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