beyond-all-reason / spring

A powerful free cross-platform RTS game engine
https://beyond-all-reason.github.io/spring/
Other
210 stars 99 forks source link

Units do not move out of the way of construction when edges of constuction is blocked #1691

Open Beherith opened 1 week ago

Beherith commented 1 week ago

I have found a simple, consistent, and deterministically reproducible way to see this bug: It happens when and only when:

The queued building only contains yellow blocking squares on any of the rightmost column of small-squares:

image

if all blocked squares are only within the rightmost single column of squares, then the commander does not receive a command to move out of the way and is blocked indefinitely.

image

Also happens with luaui/luarules disable, doesnt matter what structure you place.

armck (construction bot) and armcv (construction vehicle) and armfark are not affected by this bug. If another unit queues a structure with the last column overlapping an armcom, then the armcom never moves out of the way:

image

armch (construction hovercraft) gets no move out of the way command on both RIGHT column blocking, or BOTTOM row blocking.

image

Raised by quite a few people on discord: https://discord.com/channels/549281623154229250/724924957074915358/1287439088076460065

sprunk commented 1 week ago

Some remarks by various people:

lostsquirrel1 commented 1 week ago

Is there a simple replay I can look at that shows this issue early on?

Beherith commented 1 week ago

This replay shows it on the second batch of queued wind farms: https://discord.com/channels/549281623154229250/1286802236650815551/1287373051683541013

lostsquirrel1 commented 1 week ago

So this replay then? https://bar-rts.com/replays/13fdef6628d75c15d79c42c1c5930e5d

Beherith commented 1 week ago

Engine 2555 affected Engine 2511 affected!

Beherith commented 1 week ago

BAR dac88466c4cf4b1cdc3a6738aafad3c5d06092c9 UNAFFECTED BAR 1f5cf3cca3eee01d58715a92d7372241302e1c11 FIRST AFFECTED : https://github.com/beyond-all-reason/Beyond-All-Reason/commit/1f5cf3cca3eee01d58715a92d7372241302e1c11

Beherith commented 1 week ago

So this replay then? https://bar-rts.com/replays/13fdef6628d75c15d79c42c1c5930e5d

Yes indeed, but it turns out that a gameside change triggered the issue, namely a difference in collision volume or movedef footprint. Latest BAR and any engine allows you to repro it in 2 seconds, but moving commander and then queueing a building to the left of it with just a single column overlap of footprints.

Beherith commented 1 week ago

Note that I pushed a hotfix to the game: https://github.com/beyond-all-reason/Beyond-All-Reason/commit/c0901712c81c6e951e37cd470d557eb803602238 Which changes comm movedef footprint to 3

drivver44 commented 1 week ago

note from me i dont see a need for bar to have a missmatch between footprint size and movedeff sizing in general. im fairly sure that there are a number of units in bar that have this issue that have not been reported. especially with the fact that i did adjust a number of movedeffs on units about two years ago.

i will be going through all of bars units and make changes to units that i find that have a mismatch on footprint vs movedeff sizing at some point and time

sprunk commented 1 week ago

Engine should probably still handle this somehow. Either make units move according to the appropriate footprint or adjust mismatched footprint

lostsquirrel1 commented 1 week ago

Are there any good reasons for the unit foot print to be different from the move type foot print?

lostsquirrel1 commented 1 week ago

Otherwise we can force align the unit footprint to the movedef's footprint.

sprunk commented 1 week ago

There are some possible reasons to have them different. Most of them seem weak so I think it's fine to change though.

1) adjust the size of the shape drawn on the ground which marks units that are selected. The built-in engine square does this, and AFAICT most existing selection widgets do so as well. image

2) mobile units can be built by constructors, same as buildings. You can make a unit bigger to force some free space around it. Your fix #1694 seems to only adjust the size of individual units after they receive a movedef, and doesn't change the size of the whole type, so this should keep working as-is. image

3) default engine Shift+Ctrl modifier for construction is to surround hovered unit, this uses footprint. This is just UI so games can reimplement it if they want, and it isn't that useful for mobiles. image

4) maybe others? I'm going off the top of my head but anything I don't remember probably isn't too important. Maybe @GoogleFrog would want to say something.

GoogleFrog commented 1 week ago

It would be nice to modify mobile-mobile repulsion radius and profile (ie the force-distance function). But I would only use it and expect it to make sense when used to differentiate units within a pathfinder size. I made a PR ages ago with a multiplier for unit pushing size.

drivver44 commented 1 week ago
  1. adjust the size of the shape drawn on the ground which marks units that are selected. The built-in engine square does this, and AFAICT most existing selection widgets do so as well.

in this case i think it doesnt make sense for the footprint to be smaller than the movedeff. the selection square should reflect the area that the unit occupies. this gives the player to have the GUI reflect what is going on at the technical level without needing debuger tools or having to place a building to see the occupancy.

3. default engine Shift+Ctrl modifier for construction is to surround hovered unit, this uses footprint. This is just UI so games can reimplement it if they want, and it isn't that useful for mobiles.

footprints in bar go to as far as needed to cover the buildings. units shouldnt be exempt from this.

Are there any good reasons for the unit foot print to be different from the move type foot print?

IMO no. but i have not really seen use cases where this would be benificial.