FAForever
Add feature to preview adjacent units when building
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
Example of dynamic scaling:
https://github.com/FAForever/fa/assets/15778155/48d6bfe1-6730-4d3a-b6ff-c18d0138109c
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.
otherBpinOnDetectAdjacencyBonus(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.
@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.
Also it's better to use skirt size, currently T1 PD and walls have the same icon size as a mex.
Processed the skirt size feedback in https://github.com/FAForever/fa/pull/6283/commits/5bd048c25b821d75a9729cbeaf93dec76fbf127d
Behavior at this moment:
https://github.com/FAForever/fa/assets/15778155/1145b08a-bef2-4ebf-9812-be32efa115bc
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?
Are you still planning to continue this? After reading the discussion again I wonder if this feature is not more suited as a UI mod...
I think this would be a great feature, suitable for the base game and not a UI mod, if it got polished up (e.g. only showed for relevant buildings, T1 PD skirt size was handled properly, it showed up for planned buildings...) and if the way it was communicated was changed. Maybe something more subtle like highlighting the edge that's adjacent?
It also occurs to me that there's a few different ways that adjacency works, so it might be nice to somehow communicate that power generators next to artillery will make that building shoot faster, but next to energy storage will make this building produce more, while next to a factory will make it consume less.
I wonder if there's an easy way to show that a building is going to be rebuilt on a wreck instead of reclaiming what's under it and building the whole thing. It can get really annoying if you get that wrong.
I've looked whether we can include more context to understand if a buff is applied, and if so what type of buff would be applied. For example, whether there would be an energy consumption reduction. And therefore we could show an energy icon.
All of these type of buffs are defined here:
- https://github.com/FAForever/fa/blob/develop/lua/sim/AdjacencyBuffs.lua
- https://github.com/FAForever/fa/blob/develop/lua/sim/BuffDefinitions.lua
- https://github.com/FAForever/fa/blob/develop/lua/sim/CheatBuffs.lua
- https://github.com/FAForever/fa/blob/develop/lua/sim/OpBuffDefinitions.lua
When a unit checks for a buff to apply there's several steps:
- Check if the adjacent unit (that would provide the buff, like a power generator) is able to provide buffs. This is the
Blueprint.Adjacencyfield. - Map the adjacency blueprint field to the actual buff names. This is done with the
AdjacencyBuffs.luafile. There will be multiple hits, because there are different numbers for different unit sizes. - For each buff name, find the matching blueprint buff.
- For each blueprint buff that exists, check if the target unit (that would receive the buff, like a mass extractor, factory or artillery) meets the basic category check (as defined by
blueprintBuff.EntityCategory).
And here is where things get tricky:
- For each blueprint buff that exists, if it has a BuffCheckFunction, check if the target unit (that would receive the buff) passes that check.
These checks are defined in this file:
- https://github.com/FAForever/fa/blob/develop/lua/sim/AdjacencyBuffFunctions.lua
Which requires a unit reference, the type of unit reference that you only have in the simulation. This is especially relevant for the weapon related buffs, as they go over each weapon instance.
Which is where it breaks down a little bit. At the end of the day, all these functions end up checking one type of blueprint value or another. Even those of weapons are set directly from the blueprint. But I think if we refactor this, and especially the function signature, we'll be in a world of pain with regards to mod compatibility 😃 .
And if I take a step back - I think we already are in a world of pain by just importing the buff files. They reside in the sim folder. It wouldn't be strange for a mod author to import other sim related files for very specific buffs, like a buff that adds some entity. That would break our import.
Which ends my investigation. I'm not sure what is the best approach to proceed. Open to ideas on how to polish this further! Maybe @lL1l1 / @The-Balthazar have an idea since they recently worked with buffs quite a bit.
As a first step I would move the BuffDefinitions.lua import from global init to sim init. The global import is already creating functions that would error if called, since it imports the adjacency buff checks functions.
For getting buff data to the UI (since its in sim init now), you could have a shared adjacency buff definitions file, but it would lead to desyncs between Sim/UI when sim mods adjust those definitions, so instead I would sync select buffs at the start of the game (with the presumption that all adjacency buffs are initialized by then). The selected buffs would be a cleaned list of the buff tables created in AdjacencyBuffs.lua (since this is what StructureUnit uses). The cleaning step would recursively remove values that are not of type table, number, or string.
After you have the up-to-date buff tables you can add UI buff check functions based on the buff's effects that imitate the sim version but using blueprint fields if UserUnit doesn't expose the necessary info. Then finally you can use the Adjacency blueprint field of the adjacent units being previewed to find the correct buffs, check if they apply, and display their effects if they do.
so instead I would sync select buffs at the start of the game (with the presumption that all adjacency buffs are initialized by then).
Buff definitions rely on functions. You can not sync those function definitions I think.
I'm not sure about removing global state, especially if it's been there from the start. There are some mods that heavily rely on buffs, and maybe they'll error out because of that.
You can not sync those function definitions I think.
Yes, that's why I included the process of selecting specific buffs and cleaning the buff tables before syncing.
I'm not sure about removing global state, especially if it's been there from the start. There are some mods that heavily rely on buffs, and maybe they'll error out because of that.
By moving it to simInit it is moved from UI+Sim global state to Sim global state. I don't expect any mods to use the UI global state.
Closing due to inactivity. Feel free to reopen it if you pick it up again.