Fix mirage tanks veterancy insignia display to non allied players#2261
Conversation
Update CREDITS.md and docs/Whats-New.md to record a fix (by RAZER) for mirage tanks and other vehicles disguised as terrain incorrectly showing veterancy insignia to enemy players when not clearly visible.
|
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. |
Coronia
added
❓Phobos bug
|
@Coronia Could you explain why this issue is no test needed? Although this is a known issue of CnCNetYR, CnCNetYR uses Release v0.4.0.2, while on the develop branch, Starkku once had a commit 693339c to fix this issue. This commit was also cherry-picked to the Release v0.4.0.3 branch as 80bf9f9 and is currently the latest commit on that branch. Based on the above information, some data needs to be confirmed: My testing method was to place two elite mirage tanks for a house with
DevBuild#48 
commit 
commit 
commit
In summary, you fixed a bug that had already been fixed, and broke the behavior that allows players with |
Admittedly I did not test before this change on latest release but I tested with local dev build after applying the changes. I preplaced two mirage tanks on a map and set veteran status of one to veteran and one to elite, assigned to Player @ B, and loaded into the map when not allied with ai controlled player in spot 2. I could not see the insignias. I loaded in as observer with AI still in spot 2, I could not see the insignias. I loaded back in at spot 1 with the same team as the ai in spot 2 and I could see the insignias. I did not use player control settings for campaign maps. I physically assigned the units to be enemy controlled via triggers at game start |
This sounds like a check of the effectiveness of the fix after the changes in this PR, but it does not test whether the fix that existed before this PR is still effective.
I think this is something that already existed before the changes in this PR. This is Starkku's fix content in 693339c. if (pThis->IsDisguised() && !pThis->IsClearlyVisibleTo(HouseClass::CurrentPlayer) && !(isObserver
|| EnumFunctions::CanTargetHouse(RulesExt::Global()->DisguiseBlinkingVisibility, HouseClass::CurrentPlayer, pOwner)))
{
if (auto const pType = TechnoTypeExt::GetTechnoType(pThis->Disguise))
{
pTechnoType = pType;
pTechnoTypeExt = TechnoTypeExt::ExtMap.Find(pType);
pOwner = pThis->DisguisedAsHouse;
}
+ else if (pThis->Disguise->WhatAmI() == AbstractType::TerrainType && (!isObserver && !pOwner->IsAlliedWith(HouseClass::CurrentPlayer)))
+ {
+ return;
+ }
}Your version moved this newly added check ( - if (pThis->IsDisguised() && !pThis->IsClearlyVisibleTo(HouseClass::CurrentPlayer) && !(isObserver
- || EnumFunctions::CanTargetHouse(RulesExt::Global()->DisguiseBlinkingVisibility, HouseClass::CurrentPlayer, pOwner)))
+ if (pThis->IsDisguised() && !pThis->IsClearlyVisibleTo(HouseClass::CurrentPlayer) && !isObserver)
{
- if (auto const pType = TechnoTypeExt::GetTechnoType(pThis->Disguise))
+ if (!EnumFunctions::CanTargetHouse(RulesExt::Global()->DisguiseBlinkingVisibility, HouseClass::CurrentPlayer, pOwner))
{
...
}
- else if (pThis->Disguise->WhatAmI() == AbstractType::TerrainType && (!isObserver && !pOwner->IsAlliedWith(HouseClass::CurrentPlayer)))
+ else if (pThis->Disguise->WhatAmI() == AbstractType::TerrainType && !pOwner->IsAlliedWith(HouseClass::CurrentPlayer))
{
...
}
}So in the scenario you tested, it should be the same before and after the PR changes. This is the result of testing 
Before 
After |
If this was already fixed then please revert on behalf of my ignorance on the matter YR repo afaik doesn't use |
|
Maybe a YR repo or YR cncnet staff member should be brought in to help incorporate releases for a specific YR-Phobos release 🤷 |
|
Don't worry, I just want to confirm the data, because I think it has already been fixed, and |
Yeah I can do more testing and confirm |
I think CnCNetYR follows the Release versions of Phobos, so it should be consistent with the Phobos project's release progress on the Release v0.4.0.x patches. As for what new content has been added to develop, that should not be considered as CnCNetYR's task—if a Phobos fix needs to be synchronized to a patch version, the Phobos project maintainers will handle it; as for vanilla fixes, they wait for the next Release version. The issue this time is more about not having fully tested based on the latest Nightly Build of the develop branch first, thus mistakenly thinking the bug was not yet fixed. |
The issue is this bug has been fixed for 4 months now and no one besides me or @11EJDE11 give a damn enough to come here and check on things. CnCNet/cncnet-yr-client-package#819 It appears we can revert this PR and integrate the .3 release. |
No .3 release yet? |
|
Perhaps what we need is a label like "will be fixed in the next Phobos version" for use in the CnCNetYR repository? However, I think discussions related to this would be better held in the CnCNetYR repository or the corresponding channel on the CnCNet server, since this is the Phobos repository after all.
Yeah, it is still under development and has not been released yet. |
Can someone go over the latest Phobos fixes, cherry pick them to 0.4.0.3 and release it? |
|
I think I might be able to spare some time to help handle part of it. Haha, I got stuck right away. The existing commit |
|
Since the changes in this PR have been tested and confirmed to break the behavior of existing Phobos functionality, I have temporarily reverted it. If it is indeed found that any fixes only take effect after the changes in this PR and were not effective before, please report at any time. |
4 participants
Please see CnCNet/cncnet-yr-client-package#777 for reference.
Also please keep in mind on next release to include this for the YR repo(if merged).
This pull request addresses a visual bug affecting mirage tanks and other disguised vehicles. Specifically, it ensures that vehicles disguised as terrain (such as trees) do not incorrectly display veterancy insignia to enemy players when they are not clearly visible. This improves the game's visual consistency and prevents revealing information to opponents that should remain hidden.
Bug fix for disguised vehicles:
TechnoExt::DrawInsigniainBody.Visuals.cppto prevent vehicles disguised as terrain from displaying veterancy insignia to enemy players unless they are clearly visible or blinking visibility rules apply. This ensures that mirage tanks and similar units remain properly hidden.docs/Whats-New.mddescribing the fix for mirage tanks and other disguised vehicles showing insignia when not clearly visible.CREDITS.mdto credit the contributor responsible for the fix.