Skip to content

Fix BaseBuilderBotModule.LocomotorsForProducibles.#21665

Merged
PunkPun merged 1 commit into
OpenRA:bleedfrom
RoosterDragon:fix-bot-basebuilder
Dec 19, 2024
Merged

Fix BaseBuilderBotModule.LocomotorsForProducibles.#21665
PunkPun merged 1 commit into
OpenRA:bleedfrom
RoosterDragon:fix-bot-basebuilder

Conversation

@RoosterDragon
Copy link
Copy Markdown
Member

@RoosterDragon RoosterDragon commented Dec 2, 2024

Account for per-actor production (e.g. ProductionQueue) and per-player production (e.g. ClassicProductionQueue). This requires resolving the Production and ProductionQueue traits on both the producing actor, and the owning player actor.

When setting rally points, check the actor didn't die first.

Fixes #21663

@PunkPun PunkPun added this to the Next Release milestone Dec 2, 2024
Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI managed to put spawnpoint in an unreachable position

Screenshot 2024-12-02 at 22 48 03

On Morbid Aimless Poseidon, bottom left. I didn't quit gracefully so I don't have the replay 😞

@RoosterDragon
Copy link
Copy Markdown
Member Author

Based on other usage, looks like I was missing a !p.IsTraitDisabled check, added now.

Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran another AI game and managed to get a crash Attempted to get trait from destroyed object (hand 637 (not in world)) in this function. The log doesn't mention which line specifically

at OpenRA.Mods.Common.Traits.BaseBuilderBotModule.LocomotorsForProducibles(Actor producer)
at OpenRA.Mods.Common.Traits.BaseBuilderBotModule.SetRallyPoint(IBot bot, TraitPair`1 rp) in /OpenRA/OpenRA.Mods.Common/Traits/BotModules/BaseBuilderBotModule.cs:line 310

Screenshot 2024-12-08 at 11 36 40

at about 3:30 the pink ai places rallypoint in innacacible location

cnc-2024-12-08T093305Z-0.orarep.zip

@RoosterDragon
Copy link
Copy Markdown
Member Author

An IsDead check would likely solve the not in world crash.

As to the inaccessible locations, I'm not easily seeing where the production checks are going wrong on that. If anybody who is more familiar with the production aspect of things can see the missing piece please do point it out.

Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue seems to lie in multiqueue, the ProductionQueue trait being attached to the structure, while we are looking for it on the player actor.

The is dead check seems to be consistently crashing in my test games

Account for per-actor production (e.g. ProductionQueue) and per-player production (e.g. ClassicProductionQueue). This requires resolving the Production and ProductionQueue traits on both the producing actor, and the owning player actor.

When setting rally points, check the actor didn't die first.
@RoosterDragon
Copy link
Copy Markdown
Member Author

RoosterDragon commented Dec 16, 2024

Ah - there is per-player and per-actor queue setups. Okay I've updated the code to account for that and a test run in RA and TD both seems reasonable at a glance now.

Also adding a missing Disposed check which will cover us for the case when we decide a rally point needs redoing and queue it for later, but it's died a few ticks later when we actually get around to doing it.

I'm feeling better about this version, although not fully confident.

@penev92
Copy link
Copy Markdown
Member

penev92 commented Dec 18, 2024

I'm probably quite rusty at this point, but what's the use-case for the first // Player-wide production case?
ProductionQueues can be either on the production structure OR the player actor, but Production and its derivatives are always on the structures 🤔

Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose service depot doesn't produce anything 😄

Screenshot 2024-12-19 at 19 17 43

@PunkPun PunkPun merged commit 0051d6f into OpenRA:bleed Dec 19, 2024
@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Dec 19, 2024

changelog

@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Dec 19, 2024

68ad916

@RoosterDragon RoosterDragon deleted the fix-bot-basebuilder branch December 22, 2024 18:33
@RoosterDragon
Copy link
Copy Markdown
Member Author

I'm probably quite rusty at this point, but what's the use-case for the first // Player-wide production case? ProductionQueues can be either on the production structure OR the player actor, but Production and its derivatives are always on the structures 🤔

I am struggling with the usage of these traits. I probably have this wrong and thus it's dead code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exception of type System.InvalidOperationException: TypeDictionary contains multiple instances of type OpenRA.Mods.Common.Traits.ProductionInfo

3 participants