AttachmentUnit improvement - Selection and Flash by DeathFishAtEase · Pull Request #1992 · Phobos-developers/Phobos

DeathFishAtEase · 2025-12-07T14:49:23Z

Fixed the abnormal SelectionFlash issue caused by synchronization states and added a flag to pass the selection target to the Parent Unit when a Child Unit is selected.

When PassSelection=false, the Child Unit will be selected as usual, but unlike before, it can correctly reference SelectionFlash instead of being terminated after 1 frame.
When PassSelection=true, if a Child Unit is selected, it will search upward for the nearest Parent Unit within an AttachmentType where PassSelection=false (or the final caller).
- This will cause all Child Units along the tree path to flash due to the Parent Unit being selected.

Before (in the current latest version compiled three days ago):

Now :

In rulesmd.ini:

[AttachmentTypes]
+=PassSelect_False
+=PassSelect_True

[PassSelect_False]
PassSelection=false

[APOC]
Attachment0.Type=PassSelect_False
Attachment0.TechnoType=SREF
Attachment0.FLH=-512,0,208

Attachment1.Type=PassSelect_True
Attachment1.TechnoType=DLPH
Attachment1.FLH=512,0,208

[HTNK]
Attachment0.Type=PassSelect_False
Attachment0.TechnoType=DLPH
Attachment0.FLH=-512,256,208

Attachment1.Type=PassSelect_True
Attachment1.TechnoType=DLPH
Attachment1.FLH=-512,-256,208

[TNKD]
Attachment0.Type=PassSelect_True
Attachment0.TechnoType=APOC
Attachment0.FLH=256,0,208

[SREF]
Attachment0.Type=PassSelect_True
Attachment0.TechnoType=HTNK
Attachment0.FLH=256,0,208

The line with a red cross mark indicates that the selection status of a Child Unit is not passed to the Parent Unit when the Child Unit is selected, due to PassSelection=false.

Selected State Transmission Relationship Diagram

github-actions · 2025-12-07T14:55:57Z

Nightly build for this pull request:

compiled-dll-591a405af0ea9bc96570d6de65af4f38179fd86b.zip
These artifacts will expire in 90 days and will not be available for download after that time.

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.

Metadorius

looking good, couple of thoughts though

also do all the children flash because of InheritStateEffects? maybe that should be excluded from state effects if it's an UI flash and be controlled by other tags, like InheritCommands to signify they will also be commanded when you give order to the parent unit?

Metadorius · 2025-12-07T15:10:53Z

src/Misc/Hooks.AI.cpp

+	{
+		auto pType = attachment->GetType();
+		if (pType && pType->InheritStateEffects && !pType->PassSelection && attachment->Child)
+		{
+			int childFlashRemaining = attachment->Child->Flashing.DurationRemaining;
+
+			attachment->AI();
+
+			if (childFlashRemaining > 0)
+			{
+				attachment->Child->Flash(childFlashRemaining);
+			}
+		}
+		else
+		{
+			attachment->AI();
+		}
+	}


I think this should be done by amending AttachmentClass::AI instead

I originally planned to do that, but later found that directly modifying AttachmentClass::AI was somewhat difficult, so I went up and found the call point of AttachmentClass::AI to operate on it. If you have any good methods, feel free to edit it.

Metadorius · 2025-12-07T15:25:00Z

src/Ext/Techno/Hooks.Misc.cpp

+		if (pFinalTarget && pFinalTarget != pOriginal)
+		{
+			for (int i = 0; i < ObjectClass::CurrentObjects.Count; i++)
+			{
+				if (ObjectClass::CurrentObjects[i] == pOriginal)
+				{
+					int lastIndex = ObjectClass::CurrentObjects.Count - 1;
+					ObjectClass::CurrentObjects[i] = ObjectClass::CurrentObjects[lastIndex];
+					ObjectClass::CurrentObjects.Count--;
+					break;
+				}
+			}


Hmm, can't we take over the game's code before anything is actually selected and redirect it to as if we were clicking or selecting only the parent unit? Then we won't need that extra handling, supposedly.

I made some attempts, but ultimately failed. Although it is not elegant, currently it indeed is such that clicking Child will directly redirect to Parent, but I think the query here cannot be ignored, although if possible, it would indeed be better to handle it earlier.

Perhaps @TaranDahl or @NetsuNegi could help with this and the AI modification?

if (pExt->ParentAttachment && PassSelection)
{
pExt->ParentAttachment->Parent->Select();
return RETN;
}

Isn't this okay?

I'll replace the function in vtable

Metadorius · 2025-12-07T15:25:10Z

src/Ext/Techno/Hooks.Misc.cpp

+			bool parentAlreadySelected = false;
+			for (int i = 0; i < ObjectClass::CurrentObjects.Count; i++)
+			{
+				if (ObjectClass::CurrentObjects[i] == pFinalTarget)
+				{
+					parentAlreadySelected = true;
+					break;
+				}
+			}
+
+			if (!parentAlreadySelected)
+			{
+				ObjectClass::CurrentObjects.AddItem(pFinalTarget);
+			}


DeathFishAtEase · 2025-12-07T16:02:16Z

also do all the children flash because of InheritStateEffects? maybe that should be excluded from state effects if it's an UI flash and be controlled by other tags, like InheritCommands to signify they will also be commanded when you give order to the parent unit?

Indeed, that was passed by InheritStateEffects. I'm not entirely sure what your original code intended to do—for instance, whether synchronizing the flash state at that moment was intentional, since if the goal was to prevent individual flashing when directly selecting a child, it would still take effect for one frame.

Metadorius · 2025-12-07T19:53:01Z

Indeed, that was passed by InheritStateEffects. I'm not entirely sure what your original code intended to do—for instance, whether synchronizing the flash state at that moment was intentional, since if the goal was to prevent individual flashing when directly selecting a child, it would still take effect for one frame.

Well, the whole point of all those customisation tags is to allow people to make different attachment types depending on what they need. InheritStateEffects is meant so that the child gets chaosed when parent gets chaosed, IC'd with parent etc. Flash on the other hand is an UI thing that is not necessarily related. So a separate tag would make sense IMO.

NetsuNegi · 2025-12-08T06:47:26Z

I have greatly simplified the code and now replace the corresponding function in vtable.

src/Ext/Techno/Hooks.TechnoAttachment.cpp

Metadorius

looks great now, I think it would be great to also handle hover effect if we're doing PassSelection, so if you hover on a child and the parent is going to be selected -- then the HP (or whatever other UI) should show up for the parent. not sure if this should be a separate tag though

DeathFishAtEase · 2025-12-09T12:21:12Z

When PassSelection=yes, the child cannot be individually selected and given commands; it will always be redirected to the parent and then rely on InheritCommands to synchronize commands. Given this, in what scenario would we have the need to both redirect to the parent when selecting the child and to individually display the child's information before selection?

Metadorius · 2025-12-09T12:23:37Z

When PassSelection=yes, the child cannot be individually selected and given commands; it will always be redirected to the parent and then rely on InheritCommands to synchronize commands. Given this, in what scenario would we have the need to both redirect to the parent when selecting the child and to individually display the child's information before selection?

I thought of a scenario of children having individual health (can be targeted by enemies), but not able to be commanded. The use case kind of makes sense, and in this case there will be no way to see the health of the child unit.

DeathFishAtEase · 2025-12-09T12:58:31Z

I hope that by default, the information of the child is displayed but the parent is selected, which is the current behavior.
Suppose we add a tag to decide whether to display child/parent information, then should it work independently of PassSelection? In the case of independent operation, there are the following combinations:

PassSelection=true:
- DisplayChildInfo:
  Players can understand the component information while still feeling that the child and parent are integrated in terms of control feel (current default)
- DisplayParentInfo:
  The child will be a purely decorative component. In fact, this makes me feel more like an expanded selection range version of TransparentToMouse=true (the change you wanted above)
PassSelection=false:
- DisplayChildInfo:
  The child and parent are controlled separately. In many games, bosses with multiple components that need to be destroyed individually to be truly killed are often like this, and they do not need to be selected and operated by players.
- DisplayParentInfo:
  A design that displays parent information but only selects and operates the child is not very common, especially if the child needs to calculate HP separately.

Metadorius · 2025-12-09T13:05:12Z

I think the 4th combination you wrote is a niche scenario that won't be much useful, but the rest works.

Regardless, I think this could be done outside of this PR, this is good as-is.

DeathFishAtEase added 2 commits December 7, 2025 21:56

PassSelection

ae9b6d3

Flash fix

90853e0

DeathFishAtEase self-assigned this Dec 7, 2025

DeathFishAtEase added the No Documentation Needed No documentation needed whatsoever label Dec 7, 2025

DeathFishAtEase removed their assignment Dec 7, 2025

Metadorius reviewed Dec 7, 2025

View reviewed changes

replace functions in vtable

da676b5

NetsuNegi requested a review from Metadorius December 8, 2025 06:44

NetsuNegi reviewed Dec 8, 2025

View reviewed changes

src/Ext/Techno/Hooks.TechnoAttachment.cpp Outdated Show resolved Hide resolved

NetsuNegi reviewed Dec 8, 2025

View reviewed changes

src/Ext/Techno/Hooks.TechnoAttachment.cpp Outdated Show resolved Hide resolved

NetsuNegi reviewed Dec 8, 2025

View reviewed changes

src/Ext/Techno/Hooks.TechnoAttachment.cpp Outdated Show resolved Hide resolved

NetsuNegi mentioned this pull request Dec 9, 2025

Update For Selection & Flash of TechnoAttachment Phobos-developers/YRpp#53

Open

update

7de0539

Metadorius reviewed Dec 9, 2025

View reviewed changes

Metadorius merged commit f71219c into Phobos-developers:feature/techno-attachment Dec 9, 2025
6 checks passed

DeathFishAtEase deleted the feature/techno-attachment branch December 9, 2025 13:10

Uh oh!

Conversation

DeathFishAtEase commented Dec 7, 2025

Uh oh!

github-actions bot commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Metadorius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeathFishAtEase commented Dec 7, 2025

Uh oh!

Metadorius commented Dec 7, 2025

Uh oh!

NetsuNegi commented Dec 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Metadorius left a comment

Choose a reason for hiding this comment

Uh oh!

DeathFishAtEase commented Dec 9, 2025

Uh oh!

Metadorius commented Dec 9, 2025

Uh oh!

DeathFishAtEase commented Dec 9, 2025

Uh oh!

Metadorius commented Dec 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Dec 7, 2025 •

edited

Loading