Skip to content

bugfix(player): Fix transferred in-progress upgrades #2396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Stubbjax wants to merge 5 commits into
base: main
Choose a base branch
from
+79 −0
Open

bugfix(player): Fix transferred in-progress upgrades #2396

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5a61f29
bugfix: Cancel transferred in-progress upgrades the target player alr…
Stubbjax Jun 17, 2025
48b246b
bugfix: Prevent purchasing duplicates of transferred in-progress upgr…
Stubbjax Mar 4, 2026
8ccd9d3
fix: Improve VC6 relations
Stubbjax Mar 18, 2026
9fc16a4
fix: Add static cast
Stubbjax Mar 18, 2026
4b2c5d6
tweak: Use const_cast instead of (void*)
Stubbjax May 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions Generals/Code/GameEngine/Source/Common/RTS/Player.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
#include "GameLogic/Module/SpecialPowerModule.h"
#include "GameLogic/Module/SupplyTruckAIUpdate.h"
#include "GameLogic/Module/BattlePlanUpdate.h"
#include "GameLogic/Module/ProductionUpdate.h"
#include "GameLogic/VictoryConditions.h"

#include "GameNetwork/GameInfo.h"
Expand Down Expand Up @@ -1731,6 +1732,18 @@ void Player::setObjectsEnabled(AsciiString templateTypeToAffect, Bool enable)
}
}

//=============================================================================
static void cancelUpgradeInProduction(Object* obj, void* userData)
{
const UpgradeTemplate* upgradeTemplate = (const UpgradeTemplate*)userData;
Copy link
Copy Markdown

@xezon xezon Apr 23, 2026

Choose a reason for hiding this comment

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

static_cast

ProductionUpdateInterface* pui = ProductionUpdate::getProductionUpdateInterfaceFromObject(obj);

if (pui && pui->isUpgradeInQueue(upgradeTemplate))
{
pui->cancelUpgrade(upgradeTemplate);
}
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.

//=============================================================================
void Player::transferAssetsFromThat(Player *that)
{
Expand All @@ -1739,6 +1752,32 @@ void Player::transferAssetsFromThat(Player *that)
return;
}

#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @bugfix Stubbjax 03/02/2026 Cancel any in-progress player upgrades 'that'
// player currently has in progress that 'this' player already has in progress or completed.
std::vector<const UpgradeTemplate*> upgradesToCancel;
for (Upgrade* upgrade = that->m_upgradeList; upgrade; upgrade = upgrade->friend_getNext())
{
const UpgradeTemplate* upgradeTemplate = upgrade->getTemplate();

if (upgrade->getStatus() == UPGRADE_STATUS_IN_PRODUCTION
&& upgradeTemplate->getUpgradeType() == UPGRADE_TYPE_PLAYER
&& (hasUpgradeComplete(upgradeTemplate) || hasUpgradeInProduction(upgradeTemplate)))
{
upgradesToCancel.push_back(upgradeTemplate);
}
}

for (const UpgradeTemplate* upgradeTemplate : upgradesToCancel)
{
that->iterateObjects(cancelUpgradeInProduction, (void*)upgradeTemplate);
}

// TheSuperHackers @bugfix Stubbjax 03/02/2026 Ensure the in-progress upgrade mask is copied from 'that'
// player to 'this' player to prevent duplicate player upgrades being purchased.
m_upgradesInProgress.set(that->m_upgradesInProgress);
#endif

std::list<Object *> objsToTransfer;

// let's not transfer beacons
Expand Down
38 changes: 38 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/Common/RTS/Player.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2116,6 +2116,18 @@ void Player::setObjectsEnabled(AsciiString templateTypeToAffect, Bool enable)
}
}

//=============================================================================
static void cancelUpgradeInProduction(Object* obj, void* userData)
{
const UpgradeTemplate* upgradeTemplate = (const UpgradeTemplate*)userData;
Comment thread
xezon marked this conversation as resolved.
Outdated
ProductionUpdateInterface* pui = ProductionUpdate::getProductionUpdateInterfaceFromObject(obj);

if (pui && pui->isUpgradeInQueue(upgradeTemplate))
{
pui->cancelUpgrade(upgradeTemplate);
}
}

//=============================================================================
void Player::transferAssetsFromThat(Player *that)
{
Expand All @@ -2124,6 +2136,32 @@ void Player::transferAssetsFromThat(Player *that)
return;
}

#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @bugfix Stubbjax 03/02/2026 Cancel any in-progress player upgrades 'that'
// player currently has in progress that 'this' player already has in progress or completed.
std::vector<const UpgradeTemplate*> upgradesToCancel;
for (Upgrade* upgrade = that->m_upgradeList; upgrade; upgrade = upgrade->friend_getNext())
{
const UpgradeTemplate* upgradeTemplate = upgrade->getTemplate();

if (upgrade->getStatus() == UPGRADE_STATUS_IN_PRODUCTION
&& upgradeTemplate->getUpgradeType() == UPGRADE_TYPE_PLAYER
&& (hasUpgradeComplete(upgradeTemplate) || hasUpgradeInProduction(upgradeTemplate)))
{
upgradesToCancel.push_back(upgradeTemplate);
}
}

for (const UpgradeTemplate* upgradeTemplate : upgradesToCancel)
Comment thread
xezon marked this conversation as resolved.
Outdated
{
that->iterateObjects(cancelUpgradeInProduction, (void*)upgradeTemplate);
Copy link
Copy Markdown

@xezon xezon Mar 17, 2026

Choose a reason for hiding this comment

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

void* cast should be implicit

Copy link
Copy Markdown

@xezon xezon Apr 18, 2026

Choose a reason for hiding this comment

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

Was unchanged.

Copy link
Copy Markdown

@xezon xezon Apr 21, 2026

Choose a reason for hiding this comment

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

Do you plan to remove the void* cast?

Copy link
Copy Markdown
Author

@Stubbjax Stubbjax Apr 22, 2026

Choose a reason for hiding this comment

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

Is that not invalid for const pointers?

Copy link
Copy Markdown

@xezon xezon Apr 23, 2026

Choose a reason for hiding this comment

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

Use const_cast then.

Copy link
Copy Markdown
Author

@Stubbjax Stubbjax Apr 28, 2026

Choose a reason for hiding this comment

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

Isn't the (void*) effectively doing a const_cast + static_cast implicitly?

Copy link
Copy Markdown

@xezon xezon Apr 30, 2026

Choose a reason for hiding this comment

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

Yes but static_cast is not necessary to void*. All pointers implicitly convert to void pointer. The idea here is to remove void* cast because it is a C cast and it is best practice to avoid them in C++ code, because they cast every pointer with no restrictions which can be a source of error in some situations.

}

// TheSuperHackers @bugfix Stubbjax 03/02/2026 Ensure the in-progress upgrade mask is copied from 'that'
// player to 'this' player to prevent duplicate player upgrades being purchased.
m_upgradesInProgress.set(that->m_upgradesInProgress);
#endif

std::list<Object *> objsToTransfer;

// let's not transfer beacons
Expand Down
Loading