[Minor] Repair the manager after unit conversion #1650

a851903106 · 2025-05-03T11:56:08Z

After the conversion, the unit may lose the manager or retain some unnecessary ones. now they have been repaired.

In rulesmd.ini:

[SOMETECHNO]                            ; TechnoType
Convert.ResetMindControl=               ; boolean, default to false

After the unit conversion is completed, its mind control can be reset.
If Convert.ResetMindControl=no and there are no warheads in the unit that use MindControl=yes, then the controlled units that exceed the maximum value will be released.

单位在变形后会丢失管理器或是留下一些不需要的管理器，现在它们被修复了。

In rulesmd.ini:

[SOMETECHNO]                            ; TechnoType
Convert.ResetMindControl=               ; boolean, default to false

单位变形结束后可以重置其心灵控制。
如果Convert.ResetMindControl=no且单位没有任何使用了MindControl=yes的弹头，那么超出最大值的被控制单位会得到解放。

github-actions · 2025-05-03T12:00:54Z

Nightly build for this pull request:

compiled-dll-36b6a6cce08663329717bf761ce1e1f9e426cead.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.

src/Ext/Techno/Body.cpp

src/Ext/Unit/Hooks.Unload.cpp

TaranDahl · 2025-05-03T16:53:30Z

A topic that I think might need to be discussed: Is it necessary to forcefully clear its corresponding state?
In this PR, if a unit obtains a mind control weapon after conversion, then create a new manager for it. If a unit loses the mind control weapon after conversion, then forcefully release the units it has controlled and delete the manager.
Personally, I'm more inclined to only forcefully maintain those situations that might lead to a crash. That is: if a unit obtains a mind control weapon after conversion, then create a new manager for it, because if not, the game will crash. If a unit loses the mind control weapon after conversion, then do nothing, because this won't affect the game's continued operation.
I've marked this PR as needs discussion, hoping that the author and others who are following this PR can exchange their opinions on this topic.

a851903106 · 2025-05-04T05:10:21Z

I'm not sure about this, but for things like SpawnManager, I think it's necessary to delete them when they're not in use.

a851903106 · 2025-05-04T08:06:35Z

A topic that I think might need to be discussed: Is it necessary to forcefully clear its corresponding state? In this PR, if a unit obtains a mind control weapon after conversion, then create a new manager for it. If a unit loses the mind control weapon after conversion, then forcefully release the units it has controlled and delete the manager. Personally, I'm more inclined to only forcefully maintain those situations that might lead to a crash. That is: if a unit obtains a mind control weapon after conversion, then create a new manager for it, because if not, the game will crash. If a unit loses the mind control weapon after conversion, then do nothing, because this won't affect the game's continued operation. I've marked this PR as needs discussion, hoping that the author and others who are following this PR can exchange their opinions on this topic.

Well, I wrote a new tag to make it less obligatory.

CrimRecya

I'm not sure there may be also some things that need to be noted:

Turret/barrel recoil data
Cloak
Sight (If not in motion)
Bomb sight (If not in motion)
Sensors sight (If not in motion)

CrimRecya · 2025-05-06T08:20:10Z

src/Ext/Techno/Body.Update.cpp

+				pFirstPassenger->Transporter = pFoot;
+
+				if (pFirstPassenger->NextObject)
+					pFirstPassenger = abstract_cast<FootClass*>(pFirstPassenger->NextObject);


if (const auto pNextPassenger = abstract_cast<FootClass*>(pFirstPassenger->NextObject)) pFirstPassenger = pNextPassenger;

CrimRecya · 2025-05-06T08:41:03Z

src/Ext/Techno/Body.cpp

+			if (pType->Strength == pToType->Strength)
+				pThis->Health = oldHealth;


pThis->Health = Math::max(1, oldHealth * pToType->Strength / pType->Strength);

CrimRecya · 2025-05-06T08:48:30Z

src/Ext/Techno/Body.cpp

+	if (pType == pToType ||
+		pType->WhatAmI() != pToType->WhatAmI())
+	{
+		Debug::Log("Incompatible types between %s and %s\n", pThis->get_ID(), pToType->get_ID());
+		return false;
+	}


I'm not sure if Ares has already prompted this message?

CrimRecya · 2025-05-06T09:06:40Z

src/Ext/Techno/Body.Update.cpp

+					{
+						const auto pSlave = pSlaveNode->Slave;
+
+						if (pSlave && pSlaveNode->State != SlaveControlStatus::Dead)


From 0x6B0B4C, it seems that there is no reason to check pSlaveNode->State != SlaveControlStatus::Dead.

CrimRecya · 2025-05-06T09:10:32Z

src/Ext/Techno/Body.Update.cpp

+							{
+								pSlave->RegisterDestruction(pThis);
+								pSlave->UnInit();
+								pSlaveManager->LostSlave(pSlave);


pSlaveNode->Slave = nullptr; is OK. No more cycle and no more start timer.

CrimRecya · 2025-05-06T09:29:04Z

src/Ext/Techno/Body.Update.cpp

+		for (auto pSpawnNode : pSpawnManager->SpawnedNodes)
+		{
+			const auto pAircraft = pSpawnNode->Unit;
+			auto& Status = pSpawnNode->Status;
+
+			if (!pAircraft || Status == SpawnNodeStatus::Dead ||
+				Status == SpawnNodeStatus::Idle || Status == SpawnNodeStatus::Reloading)
+				continue;
+
+			pAircraft->SpawnOwner = nullptr;
+
+			if (Status == SpawnNodeStatus::TakeOff)
+			{
+				Kamikaze::Instance.Remove(pAircraft);
+				pAircraft->UnInit();
+			}
+			else
+			{
+				if (pSpawnNode->IsSpawnMissile)
+					pAircraft->ReceiveDamage(&pAircraft->Health, 0, RulesClass::Instance->C4Warhead, nullptr, true, false, pOwner);
+				else
+					pAircraft->Crash(nullptr);
+			}
+
+			pSpawnNode->Unit = nullptr;
+			Status = SpawnNodeStatus::Dead;
+			pSpawnNode->IsSpawnMissile = false;
+			pSpawnNode->SpawnTimer.Start(pSpawnManager->RegenRate);
+		}


Is there any special difference between these and pSpawnManager->KillNodes();? It's best to comment on it.

CrimRecya · 2025-05-06T09:33:33Z

src/Ext/Techno/Body.Update.cpp

+	{
+		if (!pSpawnManager || pType->Spawns != pSpawnManager->SpawnType)
+		{
+			if (pSpawnManager && pType->Spawns != pSpawnManager->SpawnType)


if (pSpawnManager)

CrimRecya · 2025-05-06T09:33:51Z

src/Ext/Techno/Body.Update.cpp

+	{
+		if (!pSlaveManager || pSlaveManager->SlaveType != pType->Enslaves)
+		{
+			if (pSlaveManager && pSlaveManager->SlaveType != pType->Enslaves)


if (pSlaveManager)

CrimRecya · 2025-05-06T09:43:48Z

src/Ext/Techno/Body.Update.cpp

+		}
+	}
+
+	auto& pAirstrike = pThis->Airstrike;


If you don't intend to clear it, you can put it in the later code.

CrimRecya · 2025-05-06T10:05:29Z

src/Ext/Techno/Body.Update.cpp

+	else if (pTemporalImUsing)
+	{
+		if (pTemporalImUsing->Target)
+			pTemporalImUsing->Detach();


pTemporalImUsing->LetGo(); may be better.

TaranDahl · 2025-05-08T04:53:12Z

Btw, I once investigated these attributes that might need to be adjusted during conversion. If you want to fully handle the problems caused by conversion, this might be helpful to you. TaranDahl@c94c168
iirc I checked CTOR, Init, and Limbo before, but not Unlimbo.

a851903106 · 2025-05-08T07:34:58Z

Btw, I once investigated these attributes that might need to be adjusted during conversion. If you want to fully handle the problems caused by conversion, this might be helpful to you. TaranDahl@c94c168 iirc I checked CTOR, Init, and Limbo before, but not Unlimbo.

Hmmm ...... I shouldn't need to transform the building.

update code.

27cf854

github-actions bot added the Minor Documentation is not required label May 3, 2025

a851903106 added 3 commits May 3, 2025 20:52

oh, why

52d138f

update again

8b00c3d

YRPP doesn't have what I need, sad.

ccc9032

TaranDahl reviewed May 3, 2025

View reviewed changes

src/Ext/Techno/Body.cpp Outdated Show resolved Hide resolved

src/Ext/Unit/Hooks.Unload.cpp Outdated Show resolved Hide resolved

TaranDahl added ❓Phobos bug Something isn't working properly Interaction Something related to interaction with other extension, program etc. Needs testing ⚙️T2 T2 maintainer review is sufficient Needs discussion labels May 3, 2025

a851903106 added 3 commits May 4, 2025 15:05

update again

618deb5

again

c1b6fb4

third

9437f11

a851903106 and others added 9 commits May 4, 2025 20:19

update

8fc318f

update now

00e6b10

update forth

4d98886

ok

380dd2e

final

eb71844

fix

44f9adc

i forget

951c5c7

fix it.

40b5dd5

Merge branch 'Phobos-developers:develop' into ManagerFix

c691363

Coronia requested a review from CrimRecya May 6, 2025 03:08

CrimRecya requested changes May 6, 2025

View reviewed changes

update

33a9738

Merge branch 'Phobos-developers:develop' into ManagerFix

52a77d5

TaranDahl removed the Needs discussion label May 8, 2025

Merge branch 'develop' into ManagerFix

e0666d4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Minor] Repair the manager after unit conversion #1650

[Minor] Repair the manager after unit conversion #1650

a851903106 commented May 3, 2025 •

edited

Loading

github-actions bot commented May 3, 2025 •

edited

Loading

TaranDahl commented May 3, 2025

a851903106 commented May 4, 2025

a851903106 commented May 4, 2025

CrimRecya left a comment

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

CrimRecya May 6, 2025

TaranDahl commented May 8, 2025

a851903106 commented May 8, 2025

		if (pType->Strength == pToType->Strength)
		pThis->Health = oldHealth;

[Minor] Repair the manager after unit conversion #1650

Are you sure you want to change the base?

[Minor] Repair the manager after unit conversion #1650

Conversation

a851903106 commented May 3, 2025 • edited Loading

github-actions bot commented May 3, 2025 • edited Loading

TaranDahl commented May 3, 2025

a851903106 commented May 4, 2025

a851903106 commented May 4, 2025

CrimRecya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TaranDahl commented May 8, 2025

a851903106 commented May 8, 2025

a851903106 commented May 3, 2025 •

edited

Loading

github-actions bot commented May 3, 2025 •

edited

Loading