Page MenuHomeWildfire Games

Use only one coordinate system for entities in rmgen
ClosedPublic

Authored by elexis on Oct 31 2017, 10:39 AM.

Details

Summary

As reported by rapidelectron in D189 and temple in #4338, the successfully placed Entities have their coordinates multiplied by CELL_SIZE to match the coordinate system used by CCmpPosition / Pathfinder.
But the SimpleObject and SimpleGroup then request the transformed location without undoing the transformation.
There should only be one coordinate system to avoid adding and undoing a transformation.
The transformation can just be done in C++ when parsing the final entities.

Test Plan

Pray to the OOS gods for the fixed point arithmetic. In LosUpdateHelperIncremental of the RangeManager I found a similar type conversion.
Change the >= 1 of object.js to >= 4 or greater to see that starting berries don't overlap anymore.

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

elexis created this revision.Oct 31 2017, 10:39 AM
Vulcan added a subscriber: Vulcan.Oct 31 2017, 10:41 AM
Executing section Default...
Executing section Source...
Executing section JS...

http://jenkins-master:8080/job/phabricator_lint/649/ for more details.

Successful build - Chance fights ever on the side of the prudent.

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

http://jenkins-master:8080/job/phabricator/2198/ for more details.

@leper the proposed fixed point division used in MapReader.cpp is platform-safe by design, right?

Removing the (int) conversion yields:

../../../source/graphics/MapReader.cpp:1386:89: error: ambiguous overload for ‘operator*’ (operand types are ‘fixed {aka CFixed<int, 2147483647, 32, 15, 16, 65536>}’ and ‘const ssize_t {aka const long int}’)

TERRAIN_TILE_SIZE is 4.

Given that there is no division there, dunno.

elexis added a comment.Nov 1 2017, 8:24 PM

*multiplication (as in operator*(int n)) from Fixed.h (which doesn't have the reassuring comment Position.h contains).
Well, nevermind, it can't go wrong (at least not more wrong than any other position computation that doesn't overflow), even if the CFixed operator* were used.

This revision was automatically updated to reflect the committed changes.