Opened 4 years ago

Closed 3 years ago

#4338 closed defect (fixed)

Large and small stone colliding (SimpleGroup SimpleObject avoidSelf check broken)

Reported by: elexis Owned by: elexis
Priority: Should Have Milestone: Alpha 23
Component: Maps Keywords:
Cc: Patch:

Description (last modified by temple)

In the mainland map of the attached replay, a small and large stone were placed in the exact same spot. Thus units got stuck trying to gather the inner one. The bug is known from multiple maps (copy n paste hell).

Attachments (2)

replay.txt (1.2 KB ) - added by elexis 4 years ago.
stone.jpg (77.1 KB ) - added by elexis 4 years ago.

Download all attachments as: .zip

Change History (11)

by elexis, 4 years ago

Attachment: replay.txt added

by elexis, 4 years ago

Attachment: stone.jpg added

comment:1 by elexis, 4 years ago

Description: modified (diff)

comment:2 by temple, 4 years ago

Description: modified (diff)

There's a big bug in placer.js. It stores the previous coordinates like this:

resultObjs.push(new Entity(this.type, player, x, z, angle));

In entity.js these are converted to meters rather than tiles:

this.position = {
	x: x * CELL_SIZE,
	y: 0,
	z: z * CELL_SIZE

But then in player.js when testing for avoidSelf it mixes the two instead of dividing the position by CELL_SIZE:

var dx = x - resultObjs[i].position.x;
var dy = z - resultObjs[i].position.z;

So, for example:

x = 60.534041579297785, z = 104.47036333882323
resultObjs pos x = 243.887196250026, z = 425.05100603813344
dx = -183.3531546707282, dy = -320.5806426993102

The avoidSelf check literally never fails.

if (dx*dx + dy*dy < 1)
	fail = true;

So we see berry bushes overlapping even though they have an avoidSelf check. (There probably should be a max radius involved here instead of always checking < 1 tile.)

For big and small stones, each SimpleObject in the SimpleGroup is checked against itself rather than also against previous things, so even if the avoidSelf check had been working we would still have this problem. And again, even if that too had been working it only checks within one tile, and large stones have a 20m = 5 tile diameter and small stones a 10m = 2.5 tile diameter, so they could still overlap. (Their obstructions are squares with sides 13m and 7m.)

[new SimpleObject(oStoneSmall, 0,2, 0,4), new SimpleObject(oStoneLarge, 1,1, 0,4)],

This stuff needs a little help. :)

comment:3 by elexis, 4 years ago

Thanks for finding that bug!

Fixing rmgen bugs implies different map generations, so all maps would look different if it were fixed (see #4294).

However in this case it might not be as drastic, since it only influences the location within a single group (I hope).

Also if I recall correctly there is some patch at phabricator that addresses the reported issue for one map.

comment:4 by elexis, 3 years ago

Keywords: simple removed

Someone was working on this already and came to exactly the same conclusions as you Phab:D189. Well investigated!

comment:5 by elexis, 3 years ago

Summary: Large and small stone collidingLarge and small stone colliding (SimpleGroup SimpleObject avoidSelf check broken)

comment:6 by elexis, 3 years ago

In 20396:

Use only one coordinate system for locations in the rmgen system.

Thereby fix the distance check of the SimpleObject, refs #4338, D189 and
remove the CELL_SIZE engine constant magic number, refs #4034.

Differential Revision:
Thanks a lot to rapidelectron and temple who independently discovered this!

comment:7 by elexis, 3 years ago

Milestone: BacklogAlpha 23

Only 5 lines copy & paste & modify from Phab:D189 missing.

comment:8 by elexis, 3 years ago

I suspect this is also the cause for nomad units that are spawned at the map border sometimes not being complete. This must be fixed for the release.

comment:9 by elexis, 3 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 21283:

Let SimpleObject also avoid other SimpleObjects within the same SimpleGroup following rP9096, refs #6.

Fixes unreachable small stone mines inside large stone mines.
Fixes #4338.

Differential Revision:
Credit rapidelectron for working out the two main issues (refs rP20396) and providing a similar patch.

Note: See TracTickets for help on using tickets.