[PATCH] Hide changes to buildings in fog-of-war

[Philip Taylor]

When a building (or tree etc) is visible in a fog-of-war region, no new information should be revealed to the player - they shouldn't see when its hitpoints change, or when it's destroyed, etc. Perhaps they shouldn't be able to select the building at all (even just to see its name).

Probably this would have to be implemented by replacing buildings with 'mirage' entities when they fall into FoW, which are clones of the original buildings with all the dynamic/interactive bits removed. (These would have to be serialised in saved games, and therefore network-synchronised, but only rendered for the appropriate player.)

We also have to deal with the case of players ordering units to attack or gather from a mirage entity - when it comes back into vision, the units should automatically switch to attacking/gathering the real entity (or realise that it's been destroyed).

[(none)]

Milestone Unclassified deleted

[historic_bruno]

Note that this problem is seen with all entities that remain in FOW, including new enemy buildings that are constructed in previously explored areas. Basically, it's a huge cheat because if you explore the map early, you see all building changes for the rest of the game (see also #1008).

[Josh Matthews]

I'm attempting to learn the ropes of 0ad development by tackling this bug. More updates as warranted; I have a half-backed patch that was sort of working before I fixed some correctness bugs and started causing crashes instead.

[Josh Matthews]

WIP patch

[Josh Matthews]

Just some proof that I haven't given up and abandoned this project yet. The following cases work correctly:

• fogged entities at the very start of the game still appear
• a fogged building that is deleted appears unchanged
• new buildings created in fogged areas do not not appear
• revealing a fogged proxy when the original entity no longer exists causes the proxy to disappear
• following orders to gather resources from a proxy transparently switches to gathering resources from the original entity when the original one is revealed
• a fogged foundation in process of being built does not reveal any further changes in hit points

Untested, but should work:

• proxied buildings don't have an AI
• ordering a proxy attacked transparently switches to attacking the original when it is revealed

Tested, doesn't work:

• a peasant building a house that is fogged for another player will continue building the house event after it appears complete - they appear to be working on the proxied entity for some reason, as the peasant stops as soon as the other player causes the finished house to be revealed

If there are further suggestions for tests I should perform, that would be super helpful.

[Kieran P]

Hey jdm. Thanks for your patch. I'm looking forward to this being completed. In the mean time, hopefully one of our other devs can review what you have so far soon and let you know of any obvious changes needed.

[Kieran P]

Hey jdm. According to your last report the patch was WIP and still had some things to test. How's progress on that coming along?

[Josh Matthews]

I haven't had time to do any further thinking about this between exams and other work.

[Kieran P]

Alright, no problem. Are you still keen to finish this up though, when you have free time?

[Kieran P]

@jdm I'm pushing this to Alpha 11, which will be due around mid July. Hopefully you'll have some type of summer break if you're in the U.S., and can work on this again if you're still wanting to finish it up. I appreciate your work on this thus far. It's one of the big things missing from multiplayer games.

[Kieran P]

Pushing back to backlog until a more complete patch is ready for review.

[Josh Matthews]

Here's a rebased version that fixes that known problem from several months ago. I would appreciate feedback on this, especially on my solution to the problem of creating fogged buildoings at game start. Since we want those to appear, but not new buildings that are created in fogged areas after the start of the game, I ended up resorting to checks involving the game turn when the building is created. If you have a better idea on how to do this, I'm all ears.

[Josh Matthews]

The other thing I'm not thrilled about with this patch is how so many places have to start checking for proxies. One particularly insidious problem I came across is the range manager - for example, units upon finishing constructing a foundation perform a range manager query for other entities nearby. This query ends up returning proxy buildings since they're owned by the player. I didn't spend enough time convincing myself that it would be fine to ignore all proxies in the range manager, so I special cased that one situation. There are likely others, so a more general solution would be nice.

[Josh Matthews]

Note to self: test whether territory boundaries change while fogged (they should not, but probably do)

[Deiz]

As you're not on IRC at the moment I'll document some issues here.

A lot of these issues stem from proxies being real entities that exist for all players. I don't think this can be worked around due to attack targets and such needing to be valid for all players. However, clearly simply not rendering proxied entities for other players is insufficient.

The first thing I noticed is that proxied entities show territory borders for all players. You can test this by walking past a player 2's civic centre, and then deleting it with player 2 afterwards. Player 2's territory borders will remain the same until player 1's proxy of the civic centre ceases to exist.

A related issue is that proxied entities provide line of sight for their owner. So, again using the above example, if player 2 deletes his civic centre and then moves all his units away, he will still be able to see using the line-of-sight the proxy entity is providing.

Some classes of foundations still seem to get created, even when fogged at creation time. Examples include walls and farm fields, possibly others. This brings me to my next issue: Proxied entities can block construction, in both an abstract and direct sense. The former occurs when you e.g. lose a civic centre and then can't build another near it until the proxied entity is destroyed. The latter occurs any time you try to build walls, farm fields, etc. within an area that is fogged (but not hidden) to your opponent.

Lastly (a small one) you're only copying health for foundations. So, e.g. if you attack an enemy's fortress, get it down to half health, and your force dies, the proxy entity will have full health.

I suppose you're already aware, but... this issue is turning out to be quite complex.

[Josh Matthews]

The latest version I've attached should fix the various problems related to territory/influence, construction obstruction, and non-foundation health. I also added the change to the range manager to not follow changes to proxy entities at all, but I'm unsure of the full scope of the effects of this. I have not figured out why some foundations show up in fogged areas yet.

[Josh Matthews]

This most recent version should have fixed the issues with farms/walls showing up when they shouldn't. They don't trigger LOS updates like most other buildings, so the proxy creation wasn't occurring.

[Josh Matthews]

Latest WIP

[Deiz]

I did some brief testing and ran into a few issues.

Foundation proxy entities still block construction, so if you observe an enemy's uncommitted (i.e. no builder has reached it yet and it has 1 health) foundation and then leave before it starts being built, your proxy entity obstructs the construction.

Territory borders aren't always updated correctly. If you get into visual range of a proxy entity that's been destroyed, the border is removed, as expected. However, entities that weren't there when you last saw the area will not update their borders until another of that player's (nearby?) visible entities with territory is finished. The delayed update might also happen upon destruction of another visible entity, but I haven't tested that.

It also seems that territory that's there at map start is always revealed. I.e. you will see the territory border of the enemy's civ centre no matter what.

Lastly (this relates somewhat to our IRC discussion), you can't see your own foundation entities if you place them in a fogged region. Nor do you get a proxy entity for them; they're simply hidden until they're within your LOS. For now I'd just create them as proxies immediately.

Restricting information leakage (allowing 'virtual' foundation placement, with the real foundation only being placed when a builder is within LOS) seems outside the scope of this patch, as it's quite complex. Beyond just that, there'd also be issues like the civ centre range restriction or structures that can only be built in friendly or neutral territory - both cases could be impacted by unseen entities.

[wraitii]

I'm thinking this could be done in an easier way and with fewer edge cases if the Proxies were components of the actual entity, instead of autonomous entities. Does anybody have input on that?

[historic_bruno]

(There was some discussion about this on IRC, see ​http://irclogs.wildfiregames.com/2013-05-05-QuakeNet-%230ad-dev.log -- from 16:14 to 18:05)

[Itms]

Hello! I've worked on a patch for this feature.

Taking in account previous discussions, I decided to design this using components. I managed to implement most of the things jdm and Deiz mentioned above, without increasing dramatically the complexity of the code.

I was not able to test it on the network yet. I am also wondering what kind of tests I should add for my components.

Finally, this patch does not address #1008, to avoid a patch too huge, but once this is OK I should be able to work on this too.

Please share your thoughts! And thanks in advance for the review work :)

[Josh Matthews]

Woo! I have to say that I like how many of the changes just relate to visibility assumptions; this definitely feels more maintainable than my earlier attempts at first glance. Thanks for taking this on :)

[Itms]

This is a new version of the patch, using EntityRenamed to improve command redirecting between mirages and real entities.

I also reverted a wrong change about the visibility states defined in the range manager. Now the "fogged" state only describes a mirage entity in the FoW, which is useful to determine, when the real entity is destroyed, when the mirage can be destroyed too.

[Josh]

Not an actual functional review, but you have trailing spaces on line 340 of TemplateLoader.cpp and line 337 of BuildingAI.js. The comment on line 338 of BuildingAI.js, and a few similar ones, don't really make sense to me (remember that comments should be understandable by anyone who reads your code). The parentheses in the if statement on line 95 unit_actions.js isn't needed (AND is always evaluated before OR). you should combine the two for loops starting on lines 71 and 74 of CCmpFogging.cpp similar to what you do in Deserialize(). You have extraneous brackets and indentation on lines 100-102 and 105-112 of CCmpFogging.cpp. There is some similar irregular indenting/bracketing in the switch on line 69 of CCmpMirage.cpp. Comment on 1370 of CCmpRangeManager.cpp doesn't make sense to me. Otherwise, it looks pretty good.

[Itms]

Thanks for your input!

This version addresses your comment and fixes some problems I came across while testing.

I'll be able to improve this by cleverly select for which entities the mirages should be updated. This will be way better performance-wise and will make possible the fogging of Gaia entities.

In the meantime, this version should be fully functional and testable. Thanks again in advance for the review work!

[Josh]

Just some quick comments as I read through it again:

• I think I was too vague about the switch statements. You have:

switch (msg.GetType()) 
{ 
case MT_Update: 
{ 
    UpdateMirages(); 
} 
break; 
case MT_Destroy: 
{ 
    for (std::vector<entity_id_t>::iterator it = m_Mirages.begin(); it < m_Mirages.end(); ++it) 
    { 
        CmpPtr<ICmpMirage> cmpMirage(GetSimContext(), *it); 
        if (cmpMirage) 
            cmpMirage->SetParent(INVALID_ENTITY); 
    } 
} 
break; 
} 

While according to the ​coding conventions it should be:

switch (msg.GetType()) 
{ 
case MT_Update: 
    UpdateMirages(); 
    break; 
case MT_Destroy: 
    for (std::vector<entity_id_t>::iterator it = m_Mirages.begin(); it < m_Mirages.end(); ++it) 
    { 
        CmpPtr<ICmpMirage> cmpMirage(GetSimContext(), *it); 
        if (cmpMirage) 
            cmpMirage->SetParent(INVALID_ENTITY); 
    } 
    break; 
} 

• Nothing seems to have actually changed besides breaking commented out code in CCmpUnitRenderer.cpp?

  source/simulation2/components/CCmpUnitRenderer.cpp

  @@ -22 +22 @@
   
   #include "simulation2/MessageTypes.h"
   
  +#include "ICmpFogging.h"
   #include "ICmpPosition.h"
   #include "ICmpRangeManager.h"
   #include "ICmpSelectable.h"
  @@ -374 +375 @@
           CmpPtr<ICmpVision> cmpVision(unit.entity);
           if (cmpVision && cmpVision->GetAlwaysVisible())
               unit.visibility = ICmpRangeManager::VIS_VISIBLE;
  +
  +        // Uncomment the following two lines to prevent the models from popping into existence
  +        // near the LOS boundary. Is rather resource intensive.
  +        //else if (cmpVision && cmpVision->GetRetainInFog())
  +        //  unit.visibility = ICmpRangeManager::VIS_VISIBLE;
  +
           else
           {
               CmpPtr<ICmpRangeManager> cmpRangeManager(GetSystemEntity());
  -            // Uncomment the following lines to prevent the models from popping into existence
  -            // near the LOS boundary. Is rather resource intensive.
  -            //if (cmpVision->GetRetainInFog())
  -            //  unit.visibility = ICmpRangeManager::VIS_VISIBLE;
  -            //else
  -                unit.visibility = cmpRangeManager->GetLosVisibility(unit.entity,
  -                    GetSimContext().GetCurrentDisplayedPlayer());
  +            unit.visibility = cmpRangeManager->GetLosVisibility(unit.entity, 
  +                GetSimContext().GetCurrentDisplayedPlayer());
           }
       }
       else

• You might want to check you updated the copyright year in all the files you modified, but otherwise it looks good stylistically.

[Itms]

Thanks again Josh! Actually, when uncommented, the UnitRenderer code failed when cmpVision didn't exist. But after some changes in my patch, this is indeed a bugfix independent of the fogging system.

This version of the patch includes fogging of Gaia entities and I cleaned up some parts of the code. Please make comments about the functionality!

[Itms]

Hey! I am struggling with the range manager for the optimization system. Above is a wip partial patch I upload, just to be able to discuss this on IRC right now.

EDIT: I'll eventually revert all my awful changes and work on a general-use system of visibility updates. Such messages might also be useful for other systems like UnitAI.

[Itms]

This is a patch adding the VisibilityChanged message. I haven't really tested it yet, but it seems to work in a game without AI enemy.

[Itms]

This is a new version of my patch, addressing a big oversight and serialization issues.

However, it does not work as expected, so I've still work to do on it. In the meantime, my fogging patch can still be reviewed!

[Itms]

This is, finally, the working version of my patch! Thanks a lot to Philip` for his help on IRC, along with Josh, leper and sanderd17.

I updated my fogging system accordingly and I upload a complete patch merging the two systems. Please share your thoughts :)

Possible ameliorations: write tests (not sure what is worth testing) and avoid the temporary vanishing of miraged entities. I have a system in mind to implement this, but I don't know if it is really necessary?..

[wraitii]

Graph of how much the update_visibility patch affects performance.

[Itms]

In 15508:

Add a VisibilityChanged message sent by the range manager whenever an entity changes visibility for any player.

This will be necessary for hiding buildings/trees/etc in fog-of-war, and may be useful for AIs and for UnitAI.

Refs #599.

[Itms]

I decided to script the Fogging component to allow access to Health components and so on. This patch does not change the functionality, it is just the same rewritten system.

Also, the patch is regenerated to be applicable to r15508.

[Itms]

With this version, the disappearing problem doesn't occur anymore!

I still have some work to do about mirage information, such as health, resource supplying, and so on. I decided not to add these components, because that breaks parts of the simulation logic (UnitAI tries to gather from mirages, etc.)

I'll add interesting, GUI-oriented data to the Mirage component itself in the next version of the patch, and then this should be ready for a new review.

[Itms]

This is the fully functional version of the patch!

I should have tested most of the possibilities, however I'm not sure how this will behave when toggling "reveal map".

Thanks for testing, please share your thoughts!

[wraitii]

Compiled nicely, works in most cases, some instances where it doesn't:

-if you place a foundation in fogged territory and it gets destroyed, you immediately know about it being destroyed. You shouldn't.

-If you order units to work on a fogged resource, and select that resource, you will lose selection focus when the resource becomes fogged. The GUI doesn't switch you to the miraged entity. Interestingly this only happens in Real->Mirage, because Mirage->Real switches properly.

-Trying to build over a fogged (invisible) entity does not work. This is probably because the obstruction map isn't updated per player or something, I'm not sure. However this should be fixed, since it can allow you to know where the enemy has expanded (though so far we also have territory borders, but that's unrelated). OTOH, you can build over mirages, since they have no obstruction.

-Territory expansion doesn't reveal entities. If you phase up, you won't see any entity in your newly fogged territory, when you probably should. It's probably because you only create mirages if entities fo from "hidden" to "visible", but "hidden" to "fogged" needs to be considered as well. This is also true of entities in fogged territory at the beginning of a game.

It seems however that proxy entities do not affect distance restrictions or things like that (ie if you build a CC, an enemy sees it, and you destroy it, and the enemy doesn't know about that, the proxy doesn't affect the "distance between CC"). So that's good.

Reveal Map changes nothing to the state of fogging, ie you return to what you should know, which in the end I'm fairly OK with.

The first issue should be an easy fix and needs to be. The second is annoying and should be fixed, sounds easy enough. The third issue isn't too bad, it's a complicated problem that we could imo live with for the time being (and it seems not too distant from the territory border issue so maybe you could fix both). It's already better than what we have now, and it doesn't stem imo from a fundamental design flaw in the patch. The fourth is possibly more annoying code wise but it's needed imo, even if we need special code to handle that.

I'll comment on the code itself later.

[Itms]

First issue solved :)

Also some commented out, forgotten code removed.

[Itms]

In 15538:

Recompute the number of LoS tiles whenever these tiles are reset (especially when loading serialized data).
Also add a verification about LoS tiles in the tests.

Fixes #2678
Refs #599

[Itms]

In 15563:

Some changes on the VisibilityUpdate message system, aimed at correcting recent OOS problems.
I still experience troubles with serialization tests but I played a few games without OOS. At least these changes will help us to debug more efficiently the remaining problems.

Fixes initialization of visibility data and update order for LoS-related systems (territory borders, etc).
Also removes a C-like array and fixes a player ID shifting.

Refs #599

[Itms]

Corrected some little problems. I created a branch here: ​https://github.com/na-Itms/0ad/tree/fogging to make it easier to develop, and I post the global patch here.

Please share your opinion. :)

[Itms]

I generate a svn patch from the branch I linked above. I think this version is a release candidate, so don't hesitate to test it!

[Itms]

Latest version of the patch, ready to be committed.

[Itms]

In 15612:

Hide changes to buildings in the fog-of-war.

To achieve this, mirage entities are created per player, to replace the real entities when these ones fall into the fog-of-war. These mirage entities are created on-the-fly, and destroyed when they get back in sight.
This depends heavily on the VisibilityChanged message added in r15508.

As a temporary adjustment, territories do not explore the map anymore when their borders change. See #2709.

Fixes #599