FreeOrion

Forums for the FreeOrion project
It is currently Sat Dec 16, 2017 10:41 pm

All times are UTC




Post new topic Reply to topic  [ 1 post ] 
Author Message
PostPosted: Thu May 16, 2013 6:25 pm 
Offline
AI Lead, Programmer
User avatar

Joined: Sat Sep 22, 2012 6:25 pm
Posts: 4390
I'm splittting this topic off the blockades thread since its fairly distinct from the other blockade issues and so that any further discussion of this doesn't clutter up the main blockades discussion. The trigger for it coming up there at all was that when a System had only Basic Visibility (fairly common) its contained objects list (planets, fleets, etc,) was getting 'too strong' of an update in the Copy() portion of updating ObjectMaps, being overwritten and losing information, particularly fleet information, which interfered with the blockades code among other things.

Geoff the Medio wrote:
This comment in Fleet.cpp confused me:
Code:
// removal from old fleet and setting of ship's m_fleet_id handled by the ship
A comment in Ship::Copy or similar functions when removing an object from a previous container might be helpful, to explain why this might be necessary.
Yes, I will comment it better. Let me start by explaining here my intent. For anyone else not so familiar with the visibility handling, & so you, Geoff, can let me know if my understanding has pertinent gaps, I'll include some extra review here. Although we may still want to implement an audit process like you had mentioned the other day, at least for a while to see if it still catching things not otherwise handled correctly, I was hoping to clean up the underlying update mechanisms so that the audit wouldn't actually have anything to catch. As part of that, I was wanting to try coming up with a somewhat more consistent approach for the update containers in general (be that a system, or a fleet, etc), and how that interacts with the update of the things that can be contained. A key requirement is that it be robust to ordering-- whether the container gets updated before the contained objects. Redundant update checking can be ok, so long as one part of updating doesn't interfere with another, but I did want to try avoiding any such redundancy.

Review
The basic overall process is that the server determines which objects are visible to which empires, and then it updates the Objectmaps for those empires by using the Clone() (initially) or Copy() method (for updates) for each visible object. The Copy() function copies certain information from the server's copy of the object into the empire's copy of the object, dependent on visibility.

The simplest situation "(A)" to deal with is when the new container, old container, and contained objects all have Partial Visibility or better; then the updating is all fairly straightforward & we just need to not be doing conflicting updates.

Another update situation "(B)" is when the old container is observed but the (formerly contained) objects and new container are not observed at all. If the old container was observed with with Partial Visibility (and until now at least for Systems, even with Basic Visibility) then its contained objects list is overwritten during Copy(), and, I believe as part of another (earlier?) stage in the visibility processing the not-now-observed-but-previously-contained objects get flagged into the Universe::EmpireStaleKnowledgeObjectIDs(client_empire_id) list, but don't otherwise get updated. That StaleKnowledge list something that hasn't been exposed to the AI yet, and I'm just now reading up on it. I think currently its only use is to block stale objects from being shown on the Galaxy Map. The changes we're currently making to System and Fleet Copy() updates probably make it unnecessary for the client to use the StaleKnowledge list for making accurate predictions of blockades but I want to think about it a bit more, and whether any other updating of the underlying objects could/should be done, but I'll pass on that for now.

The revision we just settled on for Systems with only Basic Visibility is that their contained objects list would be updated to add any objects currently visible in that situation, but that previously observed objects would not necessarily be erased from it.

Yet another update situation "(C)" is if the new container (and objects ) are observed, but not the old container, in that situation the old container (if present in the object map) could/should be updated to no longer think it contains those objects, and the objects and new container get whatever update is suitable to their visibility.

The Approach in my Revisions

Removal from old Container
In looking this code over, one thing that stood out was situation (C). For the things I've looked at so far, it looked like the old container was not being updated at all (instead relying on EmpireStaleKnowledgeObjectIDs to eventually clear things up, I guess). To have an update, it needs to be handled either by the new container or the objects themselves, since the old container might not have current visibility (and therefor not be getting its own individual update). Since fleets could be visible inside of containers (Systems) or outside (on starlanes), it made sense to me to have this update triggered by the object itself rather than the container. For consistency I think that approach should be taken for all objects & containers, not just fleets & systems. Since systemID is an attribute of all UniverseObjects, I put the systemID checking & removal code into UniverseObject::Copy(). Removing Ships from their old fleet is likewise handled by Ship::Copy(). Once the Ship has made sure it is removed from any previous Fleet, it is ok to update its fleetID (that could possibly be done by new Fleet when adding this Ship, but would then need to also handle the removal process to not interfere with removal from the old Fleet, and since since not all removal can be handled by a new container I felt this approach was best). I will add a comment to this portion of both those Copy() methods, referring to this approach. I have not yet looked at Planets and Buildings, but since buildings don't go planet hopping, I think that's much less of an issue for us.

For both Fleets and Systems, if the old container has Partial Visibility then it will directly overwrite its contained objects list, which may be redundant with the above checks but should not cause any problems. **edit -- scratch this following suggestion re the extra checking -- if the objects are visible they will take care of the update with the old container, and if they are not visible then they should not be removed from the old container.** We could think about doing extra checking here in case the objects are not visible, and possibly doing some update on them to directly flag that they no longer appear in the old container, but the EmpireStaleKnowledgeObjectIDs list probably meets that same need. It seems we could perhaps instead add a 'not_currently_visible_in_container' flag to UniverseObjects to perform the same function but keeping that info with the other Object info (and also allowing it to be determined in a fashion more consistent with other object updates). Right now the blockades code has not been looking at the EmpireStaleKnowledgeObjectIDs list and so on the client side may be predicting some blockades when it (probably) should not; I'll be following up on that.

Adding to New Container (or updating within container)
Different containers have different ways of storing info about their contained objects -- Systems map orbits to sets of objectsIDs (and may nee dto update orbit for an object they already contain), whereas Fleets just keep a set of shipIDs, so it seemed best to have object insertions/updates handled by the containers. I don't think there is a way that an object could be visible & have an update without its container also having at least basic visibility and therefore getting an update.

_________________
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 1 post ] 

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group