Ticket #1334 (closed enhancement: fixed)

Opened 13 months ago

Last modified 12 months ago

[PATCH] Make allies visible on map and minimap

Reported by: rdxi Owned by: Deiz
Priority: Should Have Milestone: Alpha 11
Component: UI & Simulation Keywords: patch review
Cc:

Description (last modified by k776) (diff)

Each player still maintains their own LoS calculations. But when the game renders the LoS onscreen, combine Ally LoS (that is, LoS of all players on the same team) at render time to give one big LoS for same team members.

Eventually LoS techs will be implemented. So keep that in mind when implementing this (make sure any LoS tech would only affect the current players LoS - it shouldn't expand the LoS of allies).

Attachments

shared-los.patch (9.2 KB) - added by Deiz 12 months ago.
shared-los-2.patch (7.3 KB) - added by Deiz 12 months ago.
shared-los-3.patch (7.9 KB) - added by Deiz 12 months ago.

Change History

comment:1 Changed 13 months ago by historic_bruno

This will probably be added as part of #7.

comment:2 Changed 13 months ago by k776

  • Milestone changed from Backlog to Alpha 10

As a temp thing though, Philip believes it would be easy to render an allys line of sight along with yours. Ally == same team until diplomacy is properly implemented. Might throw this into Alpha 10 and see if there is time.

comment:3 Changed 13 months ago by k776

  • Milestone changed from Alpha 10 to Alpha 11

comment:4 Changed 12 months ago by k776

  • Priority changed from Nice to Have to Should Have
  • Description modified (diff)

comment:5 Changed 12 months ago by Deiz

  • Keywords patch review added
  • Owner set to Deiz
  • Status changed from new to assigned
  • Summary changed from Make allies visible on map and minimap to [PATCH] Make allies visible on map and minimap

I've done a (to my eyes) relatively clean implementation. Rather that modify all the LoS-checking call sites, I opted to make the low-level GetLosVisibility function smarter. It can now accept a mask (calculated at the call-site), or retrieves a mask that matches a player and all of its allies (generated and cached once per frame), such that visibility by any one of them can be checked for in one operation.

Rather importantly, GetLosRevealAll is deliberately checked only for the 'main' player in a query, such that the AI's cheating doesn't let human players see the whole map.

Making GetLosVisibility "smart" seems safe enough to me, because everything it does is read-only, and GetPercentMapExplored doesn't use that function, so the summary screen is unaffected by shared LoS.

Changed 12 months ago by Deiz

comment:6 Changed 12 months ago by k776

Hey Deiz. Thanks for your work. Someone will review it soon hopefully. Meanwhile, two quick questions:

  • We eventually want to be able to have ingame diplomacy. When that happens, does your code support LoS returning to only what the player has scouted themselves, and not being stuck with allies LoS, when the allies/teams are dissolved?
  • Does this make any more rendering passes, or does it combined LoS data before rendering? We want to keep this as fast as possible.

comment:7 Changed 12 months ago by Deiz

Since r11935 made cmpPlayer.diplomacy sane, I've removed my redundant ally status code from the GUI (which apparently would have caused out-of-sync errors in multiplayer).

Changed 12 months ago by Deiz

comment:8 Changed 12 months ago by Deiz

Here's attempt #3. Per suggestions from Philip and historicbruno, I've avoided caching every frame, and instead extended cmpRangeManager so that the LoS masks are updated (from Javascript) whenever diplomacy changes.

LoS masks are now stored in cmpRangeManager as well.

Changed 12 months ago by Deiz

comment:9 Changed 12 months ago by ben

  • Status changed from assigned to closed
  • Resolution set to fixed

In 11949:

Adds shared LOS for allied players, based on patch by Deiz/F00. Fixes #1334.

comment:10 Changed 12 months ago by ben

In 11957:

Fixes bug in percentage map explored calculation. Refs #1334

Note: See TracTickets for help on using tickets.