Page MenuHomeWildfire Games
Differential D139

Mustang error + UnitMotionFlying tests
ClosedPublic
Actions

Authored by bb on Feb 11 2017, 3:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Sep 13, 12:16 PM
Unknown Object (File)
Fri, Sep 13, 12:16 PM
Unknown Object (File)
Fri, Sep 13, 12:16 PM
Unknown Object (File)
Fri, Sep 13, 12:16 PM
Unknown Object (File)
Fri, Sep 13, 12:14 PM
Unknown Object (File)
Fri, Sep 13, 11:10 AM
Unknown Object (File)
Tue, Sep 3, 6:25 PM
Unknown Object (File)
Apr 28 2017, 2:07 PM
View All 14 Files
Subscribers
elexis
fatherbushido
Itms
leper
Vulcan
Restricted Owners Package
Restricted Owners Package

Details

Reviewers
Itms
Commits
rP19373: Fix the Mustang template. Add tests for the UnitMotionFlying component.
Summary

When summoning our mustang, we get an error because the UnitMotionFlying component doesn't have a getPassClass function. and that is called in the footprint component.

The mustang is created with typing "how do you turn this on?" while selecting a building.

This bug has already been there since a19.

Test Plan

Summon mustang without patch, see error,
Summon mustang with patch, see the absence of error.

Run tests

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bb created this revision.Feb 11 2017, 3:47 PM
bb added inline comments.
source/simulation2/components/CCmpFootprint.cpp
167 ↗(On Diff #510)

Don't like the change of this line, but UnitMotionFlying as a js file...

Vulcan added a subscriber: Vulcan.Feb 11 2017, 4:31 PM

Build is green

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

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

Harbormaster completed remote builds in B504: Diff 510.Feb 11 2017, 4:31 PM
Itms requested changes to this revision.Feb 19 2017, 5:53 PM
Itms added a subscriber: Itms.

Very nice patch! ?
I agree the change in the engine is bad and I suggested a way to do it better.

source/simulation2/components/CCmpFootprint.cpp
167 ↗(On Diff #510)

What you need is to implement GetPassabilityClass in the JS file, like you already did for GetPassabilityClassName.

I suggest calling cmpPathfinder.GetPassabilityClass in Init and storing the result in a JS attribute, similarly to what is done in CCmpUnitMotion in the engine. Then GetPassabilityClass would return that stored value.

This revision now requires changes to proceed.Feb 19 2017, 5:53 PM
bb added inline comments.Feb 22 2017, 3:16 PM
source/simulation2/components/CCmpFootprint.cpp
167 ↗(On Diff #510)

I thought of doing so, but I seem not be able to call that pathfinder function from js, i.o. Engine.QueryInterface(this.entity, IID_Pathfinder) and Engine.QueryInterface(SYSTEM_ENTITY, IID_Pathfinder) results in a null object. Any idea how to call the pathfinder function?

fatherbushido added a subscriber: fatherbushido.Feb 22 2017, 3:51 PM
fatherbushido added inline comments.
source/simulation2/components/CCmpFootprint.cpp
167 ↗(On Diff #510)

ICmpPathfinder.cpp

bb updated this revision to Diff 575.Feb 22 2017, 4:52 PM
bb edited edge metadata.

That helped, changed the footprint back, and adding test for this new function. Leaving the getPassCLassName function for avoiding future bugs (it is unused now).
Updating the hawk template, as that was forgotten in earlier diff.

Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Feb 22 2017, 4:52 PM
Vulcan added a comment.Feb 22 2017, 5:59 PM

Build is green

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

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

Harbormaster completed remote builds in B566: Diff 575.Feb 22 2017, 5:59 PM
Itms requested changes to this revision.Mar 8 2017, 12:26 PM

A few bits that should be fixed but it's looking good! Sorry for the delay in reviewing...

binaries/data/mods/public/simulation/components/UnitMotionFlying.js
41 ↗(On Diff #575)

missing space here

binaries/data/mods/public/simulation/components/tests/test_UnitMotionFlying.js
10 ↗(On Diff #575)

Maybe write it with bit-shifting notation so it's clearer and easier to adapt if the classes change.

13 ↗(On Diff #575)

This should be cmpUnitMotionFlying.

This revision now requires changes to proceed.Mar 8 2017, 12:26 PM
bb updated this revision to Diff 738.Mar 8 2017, 2:35 PM
bb edited edge metadata.

Fixing comments above, also removing a trailing comma and adding a newline.

bb updated this revision to Diff 744.Mar 10 2017, 12:50 AM

years

Vulcan added a comment.Mar 10 2017, 1:34 AM

Build is green

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

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

Harbormaster completed remote builds in B714: Diff 744.Mar 10 2017, 1:34 AM
Itms accepted this revision.Apr 3 2017, 11:51 AM
This revision is now accepted and ready to land.Apr 3 2017, 11:51 AM
Closed by commit rP19373: Fix the Mustang template. Add tests for the UnitMotionFlying component. (authored by Itms). · Explain WhyApr 3 2017, 11:58 AM
This revision was automatically updated to reflect the committed changes.
fatherbushido added a comment.Apr 3 2017, 12:01 PM

(I just add a useless formal comment)

binaries/data/mods/public/simulation/components/tests/test_UnitMotionFlying.js
43 ↗(On Diff #744)

({ "x": 50, "y": 100 }) can do the job too

44 ↗(On Diff #744)

same

45 ↗(On Diff #744)

same

elexis added a subscriber: elexis.Apr 4 2017, 5:57 AM

Thanks for the patch bb! (There are also towers and runway in atlas, but no map? Sounds like something for a demo map)

leper added a subscriber: leper.Apr 4 2017, 10:39 PM

There are also towers and runway in atlas, but no map? Sounds like something for a demo map

Can I propose that we name it Flight_demo_2.{xml,pmp}?

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
simulation/
components/
14 lines
tests/
146 lines
templates/
gaia/
1 line
other/
1 line
source/
simulation2/
components/
3 lines

Diff 1088

View Options

ps/trunk/binaries/data/mods/public/simulation/components/UnitMotionFlying.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/tests/test_UnitMotionFlying.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/templates/gaia/fauna_hawk.xml

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/templates/other/plane.xml

Loading...
View Options

ps/trunk/source/simulation2/components/ICmpPathfinder.cpp

Loading...
Wildfire Games · Phabricator