Page MenuHomeWildfire Games

Add GetMapSize to cmpTerrain
ClosedPublic

Authored by Sandarac on May 30 2017, 5:01 AM.

Details

Summary

There was no way to directly get the mapsize previously, which was especially frustrating when creating trigger scripts. This will be useful for D380, and any other trigger scripts that will need to get the mapsize directly.

Test Plan

Verify by reading that no functionality has changed.

Diff Detail

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

Event Timeline

Sandarac created this revision.May 30 2017, 5:01 AM
Sandarac updated this revision to Diff 2315.May 30 2017, 5:30 AM

Some tweaks.

Vulcan added a subscriber: Vulcan.May 30 2017, 5:30 AM

Build has FAILED

Updating workspaces.
Build (release)...
In file included from ../../../source/simulation2/components/tests/test_Position.cpp:17:0:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h: In member function ‘void TestCmpPosition::test_basic()’:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:44:15: error: cannot declare variable ‘terrain’ to be of abstract type ‘MockTerrain’
   MockTerrain terrain;
               ^
In file included from /mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:18:0,
                 from ../../../source/simulation2/components/tests/test_Position.cpp:17:
../../../source/simulation2/system/ComponentTest.h:180:7: note:   because the following virtual functions are pure within ‘MockTerrain’:
 class MockTerrain : public ICmpTerrain
       ^
In file included from ../../../source/simulation2/system/ComponentTest.h:25:0,
                 from /mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:18,
                 from ../../../source/simulation2/components/tests/test_Position.cpp:17:
../../../source/simulation2/components/ICmpTerrain.h:55:14: note: 	virtual u32 ICmpTerrain::GetMapSize() const
  virtual u32 GetMapSize() const = 0;
              ^
In file included from ../../../source/simulation2/components/tests/test_Position.cpp:17:0:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h: In member function ‘void TestCmpPosition::test_serialize()’:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:116:15: error: cannot declare variable ‘terrain’ to be of abstract type ‘MockTerrain’
   MockTerrain terrain;
               ^
make[1]: *** [obj/test_Release/test_Position.o] Error 1
make: *** [test] Error 2
make: *** Waiting for unfinished jobs....

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

Sandarac updated this revision to Diff 2316.May 30 2017, 5:45 AM

Fix that.

elexis added inline comments.May 30 2017, 5:49 AM
source/simulation2/components/ICmpTerrain.h
56

Not.

GetTilesPerSide returns a number of tiles.

that is multiplied with TERRAIN_TILE_SIZE which is:

/// metres [world space units] per tile in x and z

So it actually returns metres (not even meters :-S)

Build has FAILED

Updating workspaces.
Build (release)...
In file included from ../../../source/simulation2/components/tests/test_Position.cpp:17:0:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h: In member function ‘void TestCmpPosition::test_basic()’:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:44:15: error: cannot declare variable ‘terrain’ to be of abstract type ‘MockTerrain’
   MockTerrain terrain;
               ^
In file included from /mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:18:0,
                 from ../../../source/simulation2/components/tests/test_Position.cpp:17:
../../../source/simulation2/system/ComponentTest.h:180:7: note:   because the following virtual functions are pure within ‘MockTerrain’:
 class MockTerrain : public ICmpTerrain
       ^
In file included from ../../../source/simulation2/system/ComponentTest.h:25:0,
                 from /mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:18,
                 from ../../../source/simulation2/components/tests/test_Position.cpp:17:
../../../source/simulation2/components/ICmpTerrain.h:58:14: note: 	virtual u32 ICmpTerrain::GetMapSize() const
  virtual u32 GetMapSize() const = 0;
              ^
In file included from ../../../source/simulation2/components/tests/test_Position.cpp:17:0:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h: In member function ‘void TestCmpPosition::test_serialize()’:
/mnt/data/jenkins-phabricator/workspace/phabricator/source/simulation2/components/tests/test_Position.h:116:15: error: cannot declare variable ‘terrain’ to be of abstract type ‘MockTerrain’
   MockTerrain terrain;
               ^
make[1]: *** [obj/test_Release/test_Position.o] Error 1
make: *** [test] Error 2
make: *** Waiting for unfinished jobs....

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

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/1415/ for more details.

elexis accepted this revision.May 31 2017, 6:35 PM

Every header date is updated already.
Only fixing the comment to say meters.
Thanks for the patch!

source/simulation2/components/CCmpTerrain.cpp
105

About the type u32:

Fun how a source/graphics/ constant is used in source/simulation/.

/// metres [world space units] per tile in x and z
const ssize_t TERRAIN_TILE_SIZE = 4;

ssize_t is defined as

signed integral types. The type ssize_t is capable of storing values at least in the range [-1, SSIZE_MAX].

https://www.gnu.org/software/libc/manual/html_node/General-Limits.html
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html

Seems bad to use such a type in the simulation.

u32 seems fine to me sinc GetTilesPerSide is u16, and TERRAIN_TILE_SIZE currently has the value 4 but room for 2^16-4 other values.

107

Equation correct, names are descriptive, because GetTilesPerSide of CCmpTerrain returns

	/**
	 * Returns number of tiles per side on the terrain.
	 * Return value is always non-zero.
	 */

and TERRAIN_TILE_SIZE returns

// Terrain Constants:
/// metres [world space units] per tile in x and z
source/simulation2/components/ICmpTerrain.h
56

Of course we sometimes have meters and other times metres in the code.

This revision is now accepted and ready to land.May 31 2017, 6:35 PM
This revision was automatically updated to reflect the committed changes.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/794/display/redirect