FleetWnd question: fuel and speed to not include scrapped

[neuro]

Hi All!

I'm sorry to bother all but I'm trying to figure this out and it's not triggering for some reason. I have some screenshots here, and some code, if you have a sec to take a look.

I'm trying to fix the FleetWnd to not display the fuel and speed of scrapped ships so it's more accurate. Here's my test code modifiction:

Fleet.cpp: 451, 483

Code: Select all

double Fleet::Fuel() const
{
    if (NumShips() < 1)
        return 0.0;

    // determine fuel available to fleet (fuel of the ship that has the least fuel in the fleet)
    const ObjectMap& objects = GetMainObjectMap();
    double fuel = 0.0; //Meter::METER_MAX;
    bool isFleetScrapped = true;
    for (const_iterator ship_it = begin(); ship_it != end(); ++ship_it) {
        const Ship* ship = objects.Object<Ship>(*ship_it);
        if (!ship) {
            Logger().errorStream() << "Fleet::Fuel couldn't get ship with id " << *ship_it;
            continue;
        }
        const Meter* meter = ship->GetMeter(METER_FUEL);
        if (!meter) {
            Logger().errorStream() << "Fleet::Fuel skipping ship with no fuel meter";
            continue;
        }
        if(!ship->OrderedScrapped()) {
         fuel = fuel + 0.1;
         //fuel = std::min(fuel, meter->Current());
         isFleetScrapped = false;
      } else {
         fuel = fuel + 1.0;
      }
    }
   if(isFleetScrapped) {
      fuel = 0.0;
   }
    return fuel;
}

I'm doing this just as a visual test to show the count of scrapped + non-scrapped fleets by the fuel calculation. The purpose is, visually, I should see a fleet with 1.3 if one is scrapped and 3 not scrapped. However... it never registers "scrapped", which is odd.

Here are the screenshots:
Starting Fleet: 4 unscrapped = 0.4 fuel (for testing purposes only)


After 2 scrapped = 0.4 fuel : changing a ship to "orderedscrapped" doesn't refresh the fuel change...


Moving a scrapped into a new fleet, to test at least COUNTING ship as scrapped... still 0.1 fuel. So it NEVER seems to see a ship as scrapped or it would add 1.0 to the fuel...


Here's my Ship.cpp update (you've see this...)

Code: Select all

void Ship::SetOrderedScrapped(bool b)
{
    bool initial_status = m_ordered_scrapped;
    if (b == initial_status) return;
    m_ordered_scrapped = b;
   StateChangedSignal();
   if (Fleet* fleet = GetObject<Fleet>(this->FleetID())) {
        fleet->StateChangedSignal();
    }
}

I've done the same to MaxFuel in Fleet.cpp.

Am I confused? Even when the FleetWnd is supposed to refresh (i.e. dragging a ship into a new fleet), it still shows the ship as !orderedScrapped (because it's calculating that ship's fuel as 0.1, and not 1.0 - testing). Is my getter off? I'm seeing that getter used in SetShipIcon (FleetWnd.cpp).

What am I doing wrong do you think?

My theory is either:
a) the setter isn't setting the ordered-scrapped value properly (also, do you know how I can output debug / logger statements to XCode's debugger? I'm doing these UI tests bc I can't get the debugger in XCode goin')
b) the getter isn't getting the right value of the ship
c) the FleetDetail or FleetWnd just isn't refreshin'.

Thoughts?

B,
Dom

[strooka]

PLZ use quotations and Code quotations.
You can Upload Images to www.file-upload.net and Set the imagelink into the Forum.
Your Code is very crypthic, maybe you ve Done a logical Error.
Use variable names like it is Done in fo.
Maybe make à Diff of the changes and post it into the Forum.

[Geoff the Medio]

I think I found the problem. It wasn't anything that you (neuro) were doing or changing or could have noticed...

In Fleet::Fuel, the ships in the fleet were being looked up in the ObjectMap returned from GetMainObjectMap. On the client, this is the latest known objects of the human player. These are distinct from the visible objects to the human player, mainly because I didn't figure out a good way to set things up so that they could be all stored in a single ObjectMap and have the rest of the client-side visibility work properly.

Anyway, when an order is issued, such as a ScrapOrder, only the visible objects in the client's universe are modified, but Fleet::Fuel was looking them up in the latest known objects, which includes the visible objects, but also objects seen on previous turns that aren't visible on the current turn. The latest known objects aren't updated when orders are issued client side; only the visible objects are (which are initially copies of the visible portion of the latest known objects).

In the current situation, all the ships that were being looked up in Fleet::Fuel (from the latest known objects) weren't being updated by the order (since they weren't the visible objects), so they still reported themselves as not having been ordered scrapped, even though the visible objects had been ordered and updated.

After I modified Fleet::Fuel to remove this line:

Code: Select all

    const ObjectMap& objects = GetMainObjectMap();

and replace this line

Code: Select all

        const Ship* ship = objects.Object<Ship>(*ship_it);

with this

Code: Select all

        const Ship* ship = GetObject<Ship>(*ship_it);

Things seem to be working as expected.

The whole situation of some objects being visible and others not, and some being updated and others not on the client needs to be fixed. For now, the above change should work, although it may have a side effect of making any displayed previously-known fleets that aren't visible to the human player on the current turn appear to not have fuel contributions from ships that were but presently aren't visible. Not-currently-visible fleets aren't currently displayed to players, so this shouldn't be a problem.

[neuro]

Hi Geoff!

Thanks! I'll test that and try running it based on that. If that works, I'll submit my first bug fix to FO!

More didactic speak, about what you talked about and how the issue is resolved:

I literally spent 10 minutes reading and re-reading what you wrote... I think I sort of understand the difference between latest known objects (LKO) and human-player visible objects (HPVO) based on your description. The main difference is one of two properties, it seems:

1) is the list of objects about who controls them? or,
2) is the list of objects about whether or not they're visible?

I'm just inferring, mind you, but objects in the latter category (HPVO) would be ships the player control (of course they'd always see those and want stats on them, fuel, speed, ships, etc.) and in the former category (LKO) may be ships controlled by the AI or another player. In that case, would they be able to select that and see a FleetWnd on those fleets? If so, they of course we'd want it to show the right stats. But if it's case # 2, and the LKO are HPVO simply because they're visible (not necessarily controlled) because they're copies...then it should resolve itself. As in the "fleet::fuel and fleet::speed" function are setup to use the array / object container of ship objects... from the human's perspective. Basically, the HPVO.

But that worries me, architecturally speaking. The Fleet::Fuel (and Fleet::Speed, etc.) functions purpose is to retrieve the fuel for the entire fleet for anyone, human, AI, whatev. If, in the future, an AI were making decisions based on these values (i.e. choose the fleet with the lowest fuel to get to nearest refueling point, etc.) then is this function compromised in pulling data from only the "GetObject<Ship>(*ship_it);" and not the "GetMainObjectMap();" collection? Really, what is the difference between these collections (visibility? control? or is the local GetObject just a temporary collection that's updated with GetMainObject every turn?)

Or say there were "observers" watching the game, human non-participants that could see the entire game board. Sure they might not see real-time decisions (i.e. a player scraps / unscraps / rescraps, has coffee comes back, and unscraps), but would the results still display properly?

These are just some questions that come up in my head. I'm not tryin' to dictate overall architecture, just being careful that I don't submit code that would compromise the game later down the road. And get a better understanding of the architecture.

Lastly, is there documentation I could read to understand this? I've been trolling the wiki, and starting to get a grasp of the Universe. I guess I'm spending waaaaay too much time typing this out.

Anyway, thx all, I'll make the fix, test, and if it works, submit!

Dom

[Geoff the Medio]

I think I sort of understand the difference between latest known objects (LKO) and human-player visible objects (HPVO) based on your description. The main difference is one of two properties, it seems:

1) is the list of objects about who controls them? or,
2) is the list of objects about whether or not they're visible?

It's an issue of visibility. Visible objects are the objects (Ships, planets, buildings, fleets, systems) in the game universe that a player has vision of on the current turn. Players generally (probably always?) have visibility each turn of all objects they own (which could be called "objects they control" I suppose), but players can also have visibility of objects they own, but may also have visibility of other objects depending on what the visibilitity and detection systems determines.

I'm just inferring, mind you, but objects in the latter category (HPVO) would be ships the player control (of course they'd always see those and want stats on them, fuel, speed, ships, etc.) and in the former category (LKO) may be ships controlled by the AI or another player.

If the AI controlled ship or fleet is close enough, has low enough stealth, and the player has an object with high enough detection, AI-controlled objects and objects owned by no players could also be visible objects.

In that case, would they be able to select that and see a FleetWnd on those fleets?

Currently only visible fleets are shown on the map and in the FleetWnd. Latest known fleets that aren't visible could be shown, but that would be confusing, since there would be lots of stationary fleets shown where players had previously seen them. Latest known systems are always shown, as these don't move around, so even if they're not visible, it's reasonably safe to show them to players without misleading them. Systems that are known but not visible have fog of war scanline drawn on them, however, to indicate that they're not visible.

If so, they of course we'd want it to show the right stats. But if it's case # 2, and the LKO are HPVO simply because they're visible (not necessarily controlled) because they're copies...then it should resolve itself. As in the "fleet::fuel and fleet::speed" function are setup to use the array / object container of ship objects... from the human's perspective. Basically, the HPVO.

I'm not sure what the point of that was...

But that worries me, architecturally speaking. The Fleet::Fuel (and Fleet::Speed, etc.) functions purpose is to retrieve the fuel for the entire fleet for anyone, human, AI, whatev. If, in the future, an AI were making decisions based on these values (i.e. choose the fleet with the lowest fuel to get to nearest refueling point, etc.) then is this function compromised in pulling data from only the "GetObject<Ship>(*ship_it);" and not the "GetMainObjectMap();" collection? Really, what is the difference between these collections (visibility? control? or is the local GetObject just a temporary collection that's updated with GetMainObject every turn?)

GetMainObjectMap is a utility function I added for convenience... To explain:

The universe contains several ObjectMaps...

There is Universe::m_objects. On the server, this contains all the objects in the universe in their real current state. On the clients, this contains objects that are visible on the current turn to that client's player.

There is also Universe::m_empire_latest_known_objects. On both the server and client, this is a map from empire ID to ObjectMap containing the latest known objects for each empire. On the sever, all empires' latest known objects are stored in this map, and on the clients, generally only the client's empire's latest known objects are included, since empires don't know what the latest known objects for other empires are.

The consequence of this is that on the sever, Universe::m_objects generally contains more objects than each empire's latest known objects, since some objects in the universe aren't visible to some players. However, on the clients, the client's empier's latest known objects generally contains more objects than Universe::m_objects, since some objects the player's empire had vision of on previous turns may not be visible on this turn.

Given that, what GetMainObjectMap does is, on the server, return the Universe::m_objects map of all objects in the universe, and on the client, return the client's player's empire's latest known objects map.

The point of doing that was to ensure that the larger set of objects was checked for the requested objects when calculating something about another object.

In this case, we wanted the fuel of a fleet. If this fleet happened to be not visible to a player on the current turn, it should still be possible for the player's client to estimate the fleet's fuel, but this would require looking up all the ships that were observed to be in that fleet the last time it was visible. ie. the latest known objects in the fleet. Thus, on clients, GetMainObjectMap returns the client's player's latest known objects map.

Conversely, on the server, the game should always be able to know the real true fuel available to the fleet, and doesn't need to check what various players know about the fleet. In that case, the real true gamestate objects should be checked. THus on the server, GetMainObjectMap returns the server's true gamestate set of objects.

However, as above, only the visible objects, in Universe::m_objects, are manipulated by orders on the client, between turn updates. So, if a ship object, for example, is manipulated by an order on the client, then the latest known object of that ship won't be modified, but the visible object will be.

This lead to the situation above, where the latest known version of a ship or fleet was gotten, when the visible version was desired.

Or say there were "observers" watching the game, human non-participants that could see the entire game board. Sure they might not see real-time decisions (i.e. a player scraps / unscraps / rescraps, has coffee comes back, and unscraps), but would the results still display properly?

I'm not sure wha scenario you're evisioning here, but if there were observers in a game, they'd probably be given full visibility of every objects in the game universe.

Lastly, is there documentation I could read to understand this? I've been trolling the wiki, and starting to get a grasp of the Universe. I guess I'm spending waaaaay too much time typing this out.

There's no documentation beyond comments in code and this thread about how the latest known vs. currently visible objects are handled. The basics of the visibility system (which determines what is visible and what isn't) are discussed in the v0.3 design pad.

For many purposes, including anything that's done on the server with objects, you don't need to worry about visible vs. latest known, as the server will manipulate the "real" objects in the universe when processing the gamestate. Whether there are also latest known copies of these objects for each player generally shouldn't matter. During and after turn processing, the visibility system checks which objects are visible to which players, and update each player's latest known objects accordingly. When sending turn updates, the visible objects for each player are sent, as are the player's latest known objects.

Dealing with this isn't very difficult in the UI classes, because there it is known that the code is on the client, so that there are more objects in the latest known objects for the client's player's empire than there are in the visible objects, and it's also known that orders won't modify the latest known objects, and the

However for gamestate classes, like in Fleet, things are more complicated. Within a fleet object's code, you don't know whether the fleet is the real gamestate on the server, a visible and manipulable object on a client, or a possibly-out-of-date object copy in an empire's latest known objects. So, if a fleet needs to calculate its fuel, it can't just get ship objects using GetObject, which looks up objects in Universe::m_objects, as these ships might not exist in Universe::m_objects if the fleet is a previously-visible object on a client whose ships are not visible.

Similarly, code in the Empire class generally gets objects from the "main" object map so that it doesn't need to know whether it's running on the server and accessing the true complete gamestate, or on a client and using the latest known objects. Using just the visible objects on the client wouldn't work well because stuff that empire calculates usually needs to consider previously-known but not currently visible planets and starlane connections.

GetMainObjectMap was a workaround for this issue. On the client, it returned the latest known objects map for the client's empire, and on the server it returned the full set of objects in Universe::m_objects. This was guaranteed to find some representation of an object if it existed to the best knowledge of the server or client on which the code was running. However, due to the duplication of objects, on the client this lead to the issue in this thread, with old version of objects being returned even when visible object versions of them were available.

...

So, as this illustrates, the system of sending two separate maps of objects to players needs to be revised.

There could be a varient of ObjectMap that just acts as filter on another ObjectMap, based on visibility. Clients could use this to set up a visible objects map that is just a filtered version of the latest known objects map. There would be only one copy of each object on clients, rather than two copies of visible objects, including one in the latest known objects and one in the visible objects. This would eliminate the duplication that lead to the issues earlier in this thread.

In this case, clients would be sent their latest known objects map (which does and would contain all the visible objects, as well as objects last seen on previous turns), and sent nothing extra for the visible objects.

When looking up an object, code that doesn't know whether it's dealing with previously-visible copies or up-to-date versions of an object would just call a single GetObject function. When looking up multiple objects, a similar GetObjectMap function would be used. The same GetObject or GetObjectMap functions would work on the sever or client, so could be used in code that doesn't know where it's running (ie. gamestate stuff like fleet or empire).

On the server, GetObject would get the object from the real gamestate map of objects (Universe::m_objects).

On the client, GetObject would get objects from the empire's latest known objects. For visible objects, this would return the same object as if the search was restricted to visible objects, so that situations such as the one that started this thread wouldn't occur; there would be only a single ship object for each ship known and/or visible to a client's empire, so there would be no issues about which version of the ship was looked up by ID and whether that was the one that was acted on by orders, for example.

[neuro]

Hey Guys!

Awesome info Geoff. I'm slowly digesting it. But I think it makes sense.

I have it working now to it updates the FleetWnd if you scrap / unscrap! It's great!

However, I'm working on Speed. Specifically, Fleet::Speed. Here's what the function returns:

FleetWnd.cpp

Code: Select all

double FleetDataPanel::StatValue(const std::string& stat_name) const
{
    if (const Fleet* fleet = GetObject<Fleet>(m_fleet_id)) {
        if (stat_name == SPEED_STAT_STRING)
            return fleet->Speed();
        else if (stat_name == MeterStatString(METER_FUEL))
            return fleet->Fuel();
    }
    return 0.0;
}

Fleet.cpp

Code: Select all

double Fleet::Speed() const
{
    //Logger().debugStream() << "Fleet " << this->Name() << " has speed: " << m_speed;
    return m_speed;
}

I updated the "m_speed" calculation in Fleet.cpp in here:

Fleet.cpp (lines: 1092 - 1113)

Code: Select all

void Fleet::RecalculateFleetSpeed()
{
	if (!(m_ships.empty())) {
        bool isFleetScrapped = true;
		m_speed = MAX_SHIP_SPEED;  // max speed no ship can go faster than
        for (ShipIDSet::iterator it = m_ships.begin(); it != m_ships.end(); ++it) {
            if (const Ship* ship = GetObject<Ship>(*it)) {
				if(!ship->OrderedScrapped()) {
					if (ship->Speed() < m_speed) { 
						m_speed = ship->Speed();
					}
					isFleetScrapped = false;
				}
			}
        }
		if(isFleetScrapped) {
			m_speed = 0.0;
		}
    } else {
        m_speed = 0.0;
    }
}

However, this "recalculation" is only called when the fleet is first shown. When a "scrap order" event is fired, I think it just gets the "m_speed" and doesn't actually account for recalculating...

Now, this is a private function, so I'm assuming it has to be called from within the fleet object.
My attempts at adding "RecalculateFleetSpeed()" into the Fleet::Speed() method produced this error:
error: passing 'const Fleet' as 'this' argument of 'void Fleet::RecalculateFleetSpeed()' discards qualifiers

which, after researching, tells me that the "RecalculateFleetSpeed" method isn't a const and the Speed method IS a const.

Just to test, I tried creating my own "RecalculateFleetSpeed()" method that WAS a const method (called TestSpeedCalculate) but it says it can't manipulate the m_speed variable.

Maybe I'm coming at this problem the wrong way... If I can trigger an "update" in the fleet to recalculate the speed internally, I'd go for that. After all, my Ship.cpp, on being given the scrap order, calls "fleet->StateChangedSignal();". But I can't trace what the StateChangedSignal does to the fleet object. It has no refresh.

Any thoughts?

[Geoff the Medio]

Don't give up on IRC after once try... sometimes it takes a while for someone to check it.

I updated the "m_speed" calculation in Fleet.cpp
[...]
However, this "recalculation" is only called when the fleet is first shown. When a "scrap order" event is fired, I think it just gets the "m_speed" and doesn't actually account for recalculating...

The reason speed is stored as a member variable of Fleet, but fuel isn't, is that when human players see fleets moving around, they need to know how fast the fleet is actually moving, rather than how fast it could be moving based on the visible ships in the fleet. There could be situations where a fleet is visible due to one of its ships being visible, and that ship has a faster max speed than the fleet's actual speed. If there is another ship in the fleet that is stealthier than the visible ship, but which has a slower speed, then the fleet would move at the slower speed, but players seeing only the fast ship wouldn't know the difference. If only the visible ships' speeds are used to predict the future positions of fleets in the UI, the results will be inaccurate, leading to player confusion. So, instead the actual speed of visible fleets is given to players who can see the fleet, and the future positions along a starlane are accurately predicted.

With fuel, the actual fuel of the fleet could be inaccurate to players if not all ships in the fleet are visible, but that doesn't cause the same sort of prediction and actual position discrepancies, so isn't as much of an issue.

Now, this is a private function, so I'm assuming it has to be called from within the fleet object.

Correct, although it doesn't absolutely need to be private if there's a reason to make it public.

My attempts at adding "RecalculateFleetSpeed()" into the Fleet::Speed() method produced this error:
error: passing 'const Fleet' as 'this' argument of 'void Fleet::RecalculateFleetSpeed()' discards qualifiers

which, after researching, tells me that the "RecalculateFleetSpeed" method isn't a const and the Speed method IS a const.

Sounds correct.

Just to test, I tried creating my own "RecalculateFleetSpeed()" method that WAS a const method (called TestSpeedCalculate) but it says it can't manipulate the m_speed variable.

Right... the member variable m_const can't be modified because you're in a const method. That's essentially what const means: you can't modify the member variables of the "this" object in a const method's code.

However there is a way around this. You could make m_speed be declared mutable. This would make it possible to change its value within a const method. Since m_speed is essentially a cached value calculated from other things, it's reasonable to make it mutable. However it would probably be simpler to find the fleet of any scrapped or unscrapped ship in the ship-scrapping order execution function, and manually call the (now public) RecalculateFleetSpeed function after the scrapping is updated. Alternatively, within Ship::SetOrderedScrapped, you could look up the ship's fleet, and initiate the recalculation there. The latter is probably the best... It's similar to how an object will remove itself from a system, or a building will remove itself from a planet, or a ship will remove itself from its fleet, when the object is moved to a new location with the UniverseObject::MoveTo function.

Maybe I'm coming at this problem the wrong way... If I can trigger an "update" in the fleet to recalculate the speed internally, I'd go for that. After all, my Ship.cpp, on being given the scrap order, calls "fleet->StateChangedSignal();". But I can't trace what the StateChangedSignal does to the fleet object. It has no refresh.

Generally gamestate objects, like fleets or ships or planets, don't have "Refresh" functions. The StateChangedSignal is emitted by the Fleet object when it changes, but isn't used by any gamestate object to do anything. Rather, StateChangedSignal is the way that the gamestate objects inform anything else that's interested about changes that have already happened to the gamestate objects. This allows the UI to update after an order is issued.

So, if you need to change the state of a gamestate object, generally you'd do so by directly calling a function of that object to change it. On the clients, usually this would only be done in response to an order being issued, as that's the only way players can alter the gamestate.