Skip to content
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.

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

Fix static ghost behaviours #1831

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

saurtron
Copy link
Collaborator

@saurtron saurtron commented Dec 18, 2024

Work done

  • Add leavesRadarGhost unitdef
    • defaults to true for IsBuilderUnits
    • controls what units leave static ghosts behind
  • Add staticRadarGhost unit attribute
    • defaults to unitDef->leavesRadarGhost
    • per unit toggle for static radar ghosts
    • lua Get/SetUnitStaticRadarGhost allows changing this at any time
    • controls:
      • radar wobbling when under radar
      • leaving static ghosts when out of radar or los
    • leaves a dead ghost behind when staticRadarGhost gets disabled and the unit had a static ghost, this way no information is leaked about unit whereabouts (only if 3rd parameter to SetUnitStaticRadarGhost set to true).
      • this allows immobile units hopping on transports and looking like they're still there

Related issues

Remarks

  • This approach allows control of which units leave static live/dead ghosts, and also disabling the ghosts behavior when the unit is put in a transport for example
  • Dead ghost icons fixed at separate PR: Add dead ghost icons #1887
  • Could rename gameSetup->ghostedBuildings to staticGhosts (and export/accept also ghostedBuildings alias for now).
  • Could rename live/deadGhostBuildings to live/deadStaticGhosts since it's not special for buildings now.
    • maybe change these at a different PR to not complicate this further

Pending

  • Fix ghosts not being generated when losing radar coverage of an area.
    • not sure this is possible as part of this pr
    • might need some mechanism to facilitate drawing helpers (like defenseranges) for ghosts
      • atm this isn't handled properly at least in bar, not sure about zk or engine
        • ex: when an llt moves the range stays with the ghost, and then, if the llt new pos is spotted it won't draw a range

Reference

IsImmobileUnit -> (pathType == -1U && !canfly && speed <= 0.0f)
IsBuildingUnit -> (IsImmobileUnit() && !yardmap.empty())
IsGroundUnit   -> (pathType != -1U && !canfly)

@sprunk
Copy link
Collaborator

sprunk commented Dec 18, 2024

This just forces immobiles to have ghosts while leaving games no control. Add a new unit def tag, bool leavesBuildingGhost, and make ghosts look for that.

That tag should default to true for buildings and false for mobiles, but I dug a bit and I'm no longer sure whether that new tag should default to true for immobiles. See 30c07c5, e573bdb, and https://springrts.com/mantis/view.php?id=6127.

@saurtron saurtron force-pushed the no-drift-for-immobiles branch from bfe5b71 to f3ea435 Compare January 7, 2025 13:17
@saurtron saurtron changed the title Make all immobiles have no radar drift. Fix static ghost behaviours Jan 8, 2025
@sprunk
Copy link
Collaborator

sprunk commented Jan 9, 2025

Would be nice to split this into two PRs: small one adding the boolean tag, and the big refactor + fixing minimap icons.

@saurtron saurtron force-pushed the no-drift-for-immobiles branch from f956649 to fcbb300 Compare January 9, 2025 09:27
@saurtron saurtron mentioned this pull request Jan 9, 2025
@saurtron
Copy link
Collaborator Author

saurtron commented Jan 9, 2025

Would be nice to split this into two PRs: small one adding the boolean tag, and the big refactor + fixing minimap icons.

done, see #1887

@saurtron saurtron force-pushed the no-drift-for-immobiles branch from 3a18651 to bb41bc9 Compare January 9, 2025 10:40
@saurtron saurtron marked this pull request as ready for review January 9, 2025 11:00
@saurtron saurtron force-pushed the no-drift-for-immobiles branch from bb41bc9 to afd9a07 Compare January 9, 2025 12:38
Comment on lines +2713 to +2714
if (prevValue != unit->leavesGhost)
unitDrawer->UnitLeavesGhostChanged(unit, luaL_optboolean(L, 3, false));
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two lines should probably be part of unit->SetLeavesGhost since otherwise unit drawer can fail to be notified of a change.

*/
int LuaSyncedRead::GetUnitLeavesGhost(lua_State* L)
{
const CUnit* const unit = ParseAllyUnit(L, __func__, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably be allowed for enemy units too, since that is whose ghosts you're interested in.

rts/Rendering/Units/UnitDrawerData.cpp Outdated Show resolved Hide resolved
@@ -555,7 +562,7 @@ float3 CUnit::GetErrorVector(int argAllyTeam) const
const int atSightMask = losStatus[argAllyTeam];

const int isVisible = 2 * ((atSightMask & LOS_INLOS ) != 0 || teamHandler.Ally(argAllyTeam, allyteam)); // in LOS or allied, no error
const int seenGhost = 4 * ((atSightMask & LOS_PREVLOS) != 0 && gameSetup->ghostedBuildings && unitDef->IsBuildingUnit()); // seen ghosted building, no error
const int seenGhost = 4 * ((atSightMask & LOS_PREVLOS) != 0 && leavesGhost); // seen ghosted immobiles, no error
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ghosts are no longer tied to being an immobile.

Suggested change
const int seenGhost = 4 * ((atSightMask & LOS_PREVLOS) != 0 && leavesGhost); // seen ghosted immobiles, no error
const int seenGhost = 4 * ((atSightMask & LOS_PREVLOS) != 0 && leavesGhost); // seen ghost, no error

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unsure about this, since this is just for ghosts of "generally immobile" units.

At least I don't see how this kind of ghosts could have a meaning in the game unless the unit has some kind of "generally immobile" assumption, even if it can sometimes be broken, like when a building is moved with some exception like being transported.

Mobile ones have a different kind of ghost.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you point to the different kind of ghost for mobiles? Maybe handling for the two could be combined somehow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually, looks like the mobile ones don't get drawn by engine itself, for bar it looks like it goes through unit_ghostradar_gl4.

Copy link
Collaborator Author

@saurtron saurtron Jan 12, 2025

Choose a reason for hiding this comment

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

actually, looks like the mobile ones don't get drawn by engine itself, for bar it looks like it goes through unit_ghostradar_gl4.

I suspect they could use the same path liveGhostBuildings are using tho, they probably doing it for nothing

*
* @number unitID
* @bool leavesGhost
* @bool[opt] leaveDeadGhost leave a dead ghost behind if disabling and the unit had a live static ghost.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good if leaving a dead ghost was a standalone function. That way SetUnitLeavesGhost is atomic and you can leave dead ghosts around in other circumstances at will.

Co-authored-by: sprunk <spr.ng@o2.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants