Skip to content

Conversation

@TaranDahl
Copy link
Contributor

@TaranDahl TaranDahl commented Oct 30, 2025

@TaranDahl TaranDahl added Minor Minor feature and/or fix, not a lot of changes or they are not significant ⚙️T1 T1 maintainer review is sufficient labels Oct 30, 2025
@github-actions
Copy link

github-actions bot commented Oct 30, 2025

Nightly build for this pull request:

This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build.

@DeathFishAtEase DeathFishAtEase added the No Documentation Needed No documentation needed whatsoever label Nov 2, 2025
@TaranDahl
Copy link
Contributor Author

image

Could you please explain why there is a Hunt mission queued here?
The blame says this was changed 5 years ago by @Belonit in b0cd1e0, requested from @FS-21 .

@Belonit
Copy link
Member

Belonit commented Nov 2, 2025

Maybe @FS-21 knows

@Starkku
Copy link
Contributor

Starkku commented Nov 2, 2025

Hunt has special handling for MCV's where it seeks for free space to deploy at. Might or might not be related.

@TaranDahl
Copy link
Contributor Author

Hunt has special handling for MCV's where it seeks for free space to deploy at. Might or might not be related.

Yes, that's what I find strange. This Hunt mission is for the units generated by undeploy, so it will be deployed again immediately. I can't imagine why this is done at all.

@TaranDahl TaranDahl changed the title Revert vanilla behavior changed by e359a87 Revert vanilla behavior changed by e359a87 and b0cd1e0 Nov 3, 2025
@FS-21
Copy link
Contributor

FS-21 commented Nov 3, 2025

For the age of the commit maybe that code should be part for the mindcontrol fix when the structure was undeployed into vehicle. Why hunt mission? No idea of the details.

@FS-21
Copy link
Contributor

FS-21 commented Nov 3, 2025

image Could you please explain why there is a Hunt mission queued here? The blame says this was changed 5 years ago by @Belonit in [b0cd1e0](https://github.com/Phobos-developers/Phobos/commit/b0cd1e0ffd75094c6bdc5f78309e991571f2a240), requested from @FS-21 .

Testing that binary looks fixed. Let me test the new tag

Copy link
Contributor

@Coronia Coronia left a comment

Choose a reason for hiding this comment

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

I'm still hesitate about whether Hunt should just be removed given no one knows what it exactly does, but the rest looks fine and works well in testing

@Coronia Coronia merged commit 3f56ed1 into Phobos-developers:develop Nov 3, 2025
6 checks passed
@TaranDahl
Copy link
Contributor Author

@chaserli You may need to take a look at this PR, considering that it touches some of your code.

@TaranDahl TaranDahl deleted the UndeploysInto_Sellable branch November 3, 2025 16:25
DeathFishAtEase added a commit to DeathFishAtEase/Phobos that referenced this pull request Nov 4, 2025
DeathFishAtEase added a commit to DeathFishAtEase/Phobos that referenced this pull request Nov 4, 2025
// Move ArchiveTarget check outside Conyard check to allow generic Unsellable=no buildings to be sold
return pThis->ArchiveTarget;
const auto pTypeExt = BuildingTypeExt::ExtMap.Find(pType);
return pTypeExt->UndeploysInto_Sellable ? pThis->ArchiveTarget != nullptr : true;
Copy link
Contributor

Choose a reason for hiding this comment

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

!pTypeExt->UndeploysInto_Sellable || pThis->ArchiveTarget != nullptr;
default value being false breaks compatibility a bit. Anyway, choice is yours.

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

Labels

Minor Minor feature and/or fix, not a lot of changes or they are not significant No Documentation Needed No documentation needed whatsoever ⚙️T1 T1 maintainer review is sufficient Tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants