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

Allow buildings to assist construction #1597

Open
WatchTheFort opened this issue Jul 13, 2024 · 10 comments
Open

Allow buildings to assist construction #1597

WatchTheFort opened this issue Jul 13, 2024 · 10 comments

Comments

@WatchTheFort
Copy link
Member

Currently a building with buildpower cannot assist other construction, only perform its own i.e. be a factory.

Due to this, construction turrets are implemented as mobile units with zero speed, but this has unwanted side effects, e.g. radar ghosts do not appear in fog because they are technically not buildings.

Allow buildings with buildpower to assist other construction, supporting the full set of building unit def tags that mobile builders use.

One consequence of this is that we will need to decide what counts as factory. One option is a factory is a unit with buildpower, a buildlist, and can start construction within its own footprint. Thus, SupCom's Fatboy would count as a factory, since it builds units inside itself, but BAR's combat engineers wouldn't, since they build units beside themselves.

@sprunk
Copy link
Collaborator

sprunk commented Jul 13, 2024

Sounds like what you really want is to have the unwanted side effects fixed.

@WatchTheFort
Copy link
Member Author

Sounds like what you really want is to have the unwanted side effects fixed.

Since the implementation itself is a workaround, it would be nicer to not have to implement additional workarounds for the workaround.

@sprunk
Copy link
Collaborator

sprunk commented Jul 14, 2024

Things can be immobiles instead of true buildings purposefully so fixes should be made regardless.

@saurtron
Copy link
Collaborator

saurtron commented Jan 8, 2025

Took a look at this, and one problem I see is in how orders are given, right now factories have most/all of the non unit creation commands assigned to the built unit queue.

Having factories support the same commands would make it a bit problematic to know where the queued command needs to be put. For example, if a guard/repair command is given to the factory, should it be given to the to be built units, or to the factory itself. Not sure how gui could handle that.

@WatchTheFort
Copy link
Member Author

What about for buildings that aren't factories, i.e. construction turrets?

For UI for factories, we could split the command palette in two, one for the factory itself, one for its waypoints.

@sprunk
Copy link
Collaborator

sprunk commented Jan 8, 2025

Add CMD.OPT_PUT_ORDER_INTO_THE_OPPOSITE_QUEUE cmdoption flag which puts orders in the opposite queue compared to where they would normally end up.

@saurtron
Copy link
Collaborator

saurtron commented Jan 8, 2025

What about for buildings that aren't factories, i.e. construction turrets?

For those it wouldn't be a problem, but I think there's no building ai class atm, main distinction now is movable or factory, maybe we could introduce an immobile AI to be the parent of factory, or make some specialization of factory, or just not do anything special in that case, but probably would be good... not sure if needed at this point yet.

@saurtron
Copy link
Collaborator

saurtron commented Jan 10, 2025

I got some advances here, but would like some feedback.

I have a working implementation of factories able of guard, repair and fight commands. You can check this branch or this commit to see more or less what this needs. This is just a proof of concept implementation, so please don't roast me XD.

What this shows is basically a lot of code from Builder.cpp and BuilderCAI.cpp is needed with pretty minimal changes, just removing/changing some parts where movement is used. Adding resurrect and terraform will add even more code but shouldn't be much more difficult.

Of course all of this needs to be refactored to share code. Also, the commands should work for any buildings, not just for factories, I just merged them there to have a feeling of the difficulty.

Code now is normally shared among AIs and unit types through inheritance, and there's high coupling among unit types and cais, specially for Builder and Factory types.

The problem I see, is how the inheritance graphs (for unti types and command AIs) are constructed at the moment:

graphs

Right now, I think buildings can be using any of FactoryCAI or CommandAI for ai, and Building subtypes for unit type.

Regarding command AI, ideally, there would be some code that could be shared among FactoryCAI and BuilderCAI (and maybe a new type BuildingCAI, for buildings not requiring to be factories but wanting build command support like metal extractors).

For unit types it could be more simple since there's already a Building type that could probably be used, although a BuilderBuilding could be included between Building and Factory.

Would like to hear feedback, or ideas for other possible approaches.

--

teaser screenshot with working "building" nanoturrets (using Factory type and FactoryCAI atm).

nanofactories

Yes, they look the same :P, the only advantage atm (for nanoturrets) is after getting transported they always end in cardinal direction instead of sometimes getting slightly rotated XD.

@sprunk
Copy link
Collaborator

sprunk commented Jan 10, 2025

Extracting CBuilderCache sounds like an excellent minimal starter refactor PR that could be merged right away. See also #1888. (Haven't had the time to look much beyond these, but minimal refactors like these two would be best brought individually to the front as well)

@saurtron
Copy link
Collaborator

saurtron commented Jan 11, 2025

Since those action implementations don't seem to be extremely coupled among themselves, also metal extractor behaviour seems to be pretty loosely coupled... I'm thinking maybe trying a composition approach could be worth it, where some sort of Behaviour/Action base class is created that can be added to any CommandAI independently at runtime.

So, these are the ways I can think of, so CommandAIs could be refactored to allow for builder commands (and possibly also extractor behaviour) to be used in more CommandAIs:

  1. Restructure the single inheritance graph.
    • Might need to have support for too many uneeded things for unrelated units/commandAis.
      • For example allowing builder commands for movable builders and buildings can be tricky without also giving support to MobileAIs in general
      • Another example is the metal extractor + assist turret case without having all buildings support it too
  2. Multiple inheritance.
    • Can get messy quick and require too many class compositions
    • Not sure this is allowed due to desync concerns
  3. Composition with some sort of Behaviour/CommandAIComponent class that can be pushed into CommandAI/UnitSubclass to better be able to extend.
    • Might not be feasible or messy if behaviours prove to be too tightly coupled
    • If it works could end up being a lot more extensible and clean
  4. Allowing a unit to have several CommandAIs
    • Just adding for sake of completeness here, but don't think this would be a good approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants