Add feature to preview adjacent units when building

Garanas commented 1 week ago

Description of the proposed changes

It's always been difficult to properly understand what units are considered adjacent when you are building. With this new feature we try to alleviate this problem!

With thanks to OnDetectAdjacencyBonus in gamemain.lua we've always had the ability to detect adjacency while constructing. But up until now we did not quite do anything with that ability. Now, we render the unit icon on top of all units that are adjacent.

Note that the UI is scaled to 150% in the following preview

https://github.com/FAForever/fa/assets/15778155/e9111af6-f338-41e9-8be2-c67731bc0ba0

Testing done on the proposed changes

Spawn in structures and then create all sorts of build previews.

Additional context

The feature is enabled by default. It can be disabled in the game options at Interface -> HUD -> Show adjacent units when building.

There is no reliable solution for more advanced checks and/or drawings. This includes, but is not limited to:

Check whether there are actual adjacency benefits before drawing
Drawing of lines, or other features that rely on the position of the location of the build preview

The reason for this is because we only know what unit we're adjacent to but we do not know what unit in the build preview is adjacent to that unit. When there's only one build target this is trivial, but when we drag build and/or use a template it because much, much less trivial to understand. Therefore I opted for the simple solution to just always show the icon.

Checklist

[x] Changes are annotated, including comments where useful
[x] Changes are documented in the changelog for the next game version

Garanas commented 1 week ago

I'm open to suggestions on the name of the feature in the game options 😃 !

Garanas commented 1 week ago

Example of dynamic scaling:

https://github.com/FAForever/fa/assets/15778155/48d6bfe1-6730-4d3a-b6ff-c18d0138109c

lL1l1 commented 1 week ago

The reason for this is because we only know what unit we're adjacent to but we do not know what unit in the build preview is adjacent to that unit.

otherBp in OnDetectAdjacencyBonus(userUnit, otherBp) seems to contain the bp of the adjacent build preview, even for templates. This allows checking for adjacency bonuses.

Drawing of lines, or other features that rely on the position of the location of the build preview

There could be a special case for non-template build mode, in which the build preview is located using mouse worldpos and mimicking however the engine interprets skirts and skirt offsets onto the world grid. Thinking this through more, you can apply this analysis to UserUnits' positions and skirt data, selected template data, and mouse position to identify what userunit is touching what part of the build template.

Garanas commented 1 week ago

otherBp in OnDetectAdjacencyBonus(userUnit, otherBp) seems to contain the bp of the adjacent build preview, even for templates. This allows checking for adjacency bonuses.

You're right! I just checked again, I must have goofed up there. Would make for an interesting improvements in version 2, where we for example add an energy or mass icon if it impacts production or consumption.

There could be a special case for non-template build mode, in which the build preview is located using mouse worldpos and mimicking however the engine interprets skirts and skirt offsets onto the world grid. Thinking this through more, you can apply this analysis to UserUnits' positions and skirt data, selected template data, and mouse position to identify what userunit is touching what part of the build template.

I don't think the complexity of it is worth it. The feature is quite simple right now. That keeps it maintainable.

Garanas commented 1 week ago

@lL1l1 I've prepared the functions to allow them to be expanded in the (near) future. Could you give the pull request a review as it is right now? Then we can ship that, because I think it already adds sufficient value the way it is now.

lL1l1 commented 1 week ago

Also it's better to use skirt size, currently T1 PD and walls have the same icon size as a mex.

Garanas commented 1 week ago

Processed the skirt size feedback in https://github.com/FAForever/fa/pull/6283/commits/5bd048c25b821d75a9729cbeaf93dec76fbf127d

Garanas commented 1 week ago

Behavior at this moment:

https://github.com/FAForever/fa/assets/15778155/1145b08a-bef2-4ebf-9812-be32efa115bc

BlackYps commented 1 week ago

I think for this feature to be useful it should only show adjacency when there is an actual bonus. And then we probably should show only a mass or energy symbol. The way it is now it literally shows up on every combination of buildings which makes the feature a bit annoying. I can already see that my air factory is next to the land factory. I don't need to see the icon of the building at that moment. They are a bit hard to process anyway, so at the moment I don't see a significant benefit here.

I'd say we should only ship this feature once we have it reduced to buildings where you actually care for adjacency otherwise a lot of players will probably turn it off in the beginning and then not even experience the version 2.

Unrelated to that, this should probably point to dev iteration 3 anyway, right?

FAForever / fa