Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Beherith opened this issue Sep 22, 2024 · 15 comments · Fixed by #1694
Closed
Assignees
Labels
enhancement New feature or request

Comments

@Beherith
Copy link
Contributor

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
Copy link
Collaborator

sprunk commented Sep 22, 2024

Some remarks by various people:

  • Drivver: Commanders currently have a 3x3 footprint with a 2x2 movedeff
  • lov: buggeroff checks use unit->radius

@lostsquirrel1
Copy link
Collaborator

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

@Beherith
Copy link
Contributor Author

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

@lostsquirrel1
Copy link
Collaborator

@Beherith
Copy link
Contributor Author

Engine 2555 affected
Engine 2511 affected!

@Beherith
Copy link
Contributor Author

BAR dac88466c4cf4b1cdc3a6738aafad3c5d06092c9 UNAFFECTED
BAR 1f5cf3cca3eee01d58715a92d7372241302e1c11 FIRST AFFECTED :
beyond-all-reason/Beyond-All-Reason@1f5cf3c

@Beherith
Copy link
Contributor Author

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
Copy link
Contributor Author

Note that I pushed a hotfix to the game: beyond-all-reason/Beyond-All-Reason@c090171
Which changes comm movedef footprint to 3

@drivver44
Copy link

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
Copy link
Collaborator

sprunk commented Sep 23, 2024

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

@lostsquirrel1
Copy link
Collaborator

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

@lostsquirrel1
Copy link
Collaborator

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

@lostsquirrel1 lostsquirrel1 added bug Something isn't working enhancement New feature or request and removed bug Something isn't working labels Sep 23, 2024
@sprunk
Copy link
Collaborator

sprunk commented Sep 24, 2024

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 fix bugger off issues #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
Copy link
Collaborator

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
Copy link

  • Sprunk
    i agree with them being weak at best
  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.

  • lostsquirrel1

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.

@lhog lhog closed this as completed in #1694 Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants