Skip to content

TS Service Depot: allow sell unit when repairing.#21072

Merged
PunkPun merged 2 commits into
OpenRA:bleedfrom
dnqbob:ts-sell
Oct 21, 2023
Merged

TS Service Depot: allow sell unit when repairing.#21072
PunkPun merged 2 commits into
OpenRA:bleedfrom
dnqbob:ts-sell

Conversation

@dnqbob

@dnqbob dnqbob commented Sep 23, 2023

Copy link
Copy Markdown
Contributor

Small function from original TS.

@abcdefg30

Copy link
Copy Markdown
Member

Wasn't this a bug in the original? I know we intentionally don't allow this in RA.

@dnqbob

dnqbob commented Sep 23, 2023

Copy link
Copy Markdown
Contributor Author

It is not a bug, there is a voice line for it as UnitSold.

Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockClient.cs Outdated
Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockClient.cs Outdated
Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockClient.cs
Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockClient.cs Outdated
Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockClient.cs Outdated
Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockHost.cs Outdated
Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockHost.cs Outdated
Comment thread OpenRA.Mods.Common/Traits/GrantConditionWhenDockHost.cs Outdated
@dnqbob

dnqbob commented Sep 24, 2023

Copy link
Copy Markdown
Contributor Author

Should be all fixed and done

@PunkPun PunkPun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR mostly supersedes #20177. Could we take the change to the refinery here as well to fully replace it?

public readonly int AfterDockDuration = 0;

[Desc("Host actor type(s) leading to the condition being granted. Leave empty for allowing all hosts by default.")]
public readonly HashSet<string> DockHostNames = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These should be DockTypes instead, though they aren't exposed to the notify interface at the moment. See for example the to be replaced interface INotifyResupply

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we perhaps just exclude this from the trait for now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is not ok on excluding, I am afraid. Excluding this option will cause aircraft can be sold on helipad, while helipad can be sold as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is better that we can get the DockType exposed.

@PunkPun PunkPun added this to the Next + 1 milestone Oct 9, 2023

@PunkPun PunkPun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok DockHostNames is fine for now but it will need to be replaced before next release. This field won't be getting an update rule

@PunkPun PunkPun merged commit 1a98312 into OpenRA:bleed Oct 21, 2023
@PunkPun

PunkPun commented Oct 21, 2023

Copy link
Copy Markdown
Member

Changelog

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.

3 participants