[Civil-devel]Timing problems

I looked more thoroughly at the problem you got, Mike, fixed a few bugs in
fire.py but managed to get the same error as you got. I'm not exactly sure
where it comes from, and what can be done to remedy it.

The problem basically is that somehow two DoneCmd:s end up being sent to
the clients. The first one clears away the plan as it's supposed to do,
but the second DoneCmd tries to clear away the same plan, but fails. It
fails as there are no plans for the unit anymore. The problem here is to
determine why two DoneCmd:s end up being sent.

After reading the debug output thoroughly I found that somehow
Fire.execute() got called twice for the same unit/target. It should never
be called more than once with the current code: get in, calculate, send a
MoveCmd if needed and terminate with a DoneCmd. However, it got called one
more time, the next time updateEngine() was called in mainLoop() in
server_main_loop.py. This should not happen, and I think it's a timing
problem in checkTime() in the same file. Apparently the server changes the
current turn *before* it has a chance to execute the first DoneCmd, thus
never removing it from the unit's plans, and ends up executing it once
more. For the server this is no problem, as the plans still exists and can
be executed, but this of course leads to another DoneCmd being sent out.

Here be dragons.

Solution:

Better timing handling. I propose that the execute() method be made to
return a valud indicating wether the plan is done, instead of relying on
DoneCmd removing it on the server side too. The change would be in
the method executeUnitPlans() in server/engine.py:

   # loop over all units we have
   for unit in scenario.units.values ():
      # does the unit have any plans?
      plan = unit.getActivePlan ()

      # do we have a plan that is valid and that was not executed
      # previously this turn?
      if plan and plan.getLastExecuted () != turn:
          # yep, all ok, execute the plan
          if plan.execute ( self.outgoing ):
              # plan is done now, remove it
              unit.getPlans ().remove ( plan )

          else: 
              # not yet done, set new last execution time to make sure it
              # will not be called again this turn
              plan.setLastExecuted ( turn )


The DoneCmd is of course created and sent as before, as the clients need
it. It should however not be executed on the server side, as the plan is
already removed. So the method applyServer() on DoneCmd could just be made
empty, and more the code the removes the plan to applyClient() and
applyAI().

What do you think? It's really unbeliveably hard to make a good server
engine that works well. I'd like to still believe the system we have is
not fundamentally broken by design. It may be a little crippled and
flawed, but I still would like to believe in it...

-----------------------------------------------------------------------------
      Real children don't go hoppity-skip unless they are on drugs.
           -- Susan Sto Helit, in Hogfather (Terry Pratchett)