Complete.Org: Mailing Lists: Archives: freeciv-dev: January 2002:
[Freeciv-Dev] Re: [PATCH] Improvement effects speedup (PR#1094)
Home

[Freeciv-Dev] Re: [PATCH] Improvement effects speedup (PR#1094)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: freeciv-dev@xxxxxxxxxxx
Cc: bugs@xxxxxxxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] Improvement effects speedup (PR#1094)
From: Vasco Alexandre Da Silva Costa <vasc@xxxxxxxxxxxxxx>
Date: Mon, 14 Jan 2002 17:54:56 -0800 (PST)

On Mon, 14 Jan 2002, Ben Webb wrote:
> Actually, there are two distinct problems:
> 
> 1. update_all_effects() is currently called whenever a city_info, 
>    short_city or game_info packet is received. In reality, updates are 
>    only required if these packets add or remove improvements/Wonders.
> 2. On loading a savegame, or during a turn update, many cities may build 
>    or sell improvements. Since update_all_effects() currently updates, er, 
>    "all" effects, it makes sense to call it only once, rather than for 
>    each city which has changed its list of improvements.
> 
>       I believe the "city report dialog speedup" issue is the same as 
> (2) here, but (1) is a large contributor to the problem, and should, 

Yes it is the same as (2).

> IMHO, be applied ASAP. This can be resolved by applying the patch minus 
> the "do_deferred_effect_updates" portion, such that the try_update_effects 
> function reads

In effect-init and effect-implement you did add tests for that in many
places. I didn't notice you also added some in this patch.

> static void try_update_effects(int need_effect_update)
> {
>   if (need_effect_update) {
>     update_all_effects();
>   }
> }
> 
> If desired, I can split this patch and post it.

Yes this would be a good idea. However wouldn't it be even better for
you to add that function inside common/ and replace all instances of
update_all_effects() with tests added in effect-init and effect-implement
for try_update_effects?
Or maybe rename update_all_effects() to try_update_effects() and add that
test there?

> P.S. I set out to use the START_TURN packet to signal "all deferred effect 
>      updates should now be performed" for (2) since this seemed to be the 
>      purpose of the packet. However, I found I had to add extra handling 
>      to handle_game_state() and handle_new_year(), since the START_TURN 
>      packet does not seem to get sent under all circumstances (e.g. 
>      restarting from a savegame, reconnecting to a running game, etc.) Can 
>      the START_TURN packet be sent under these circumstances too, or would 
>      this break agents? If the latter, perhaps we could add a suitable "do 
>      all updates at start of turn" packet.

Well Raimar is the knowledgeable person in this case. What do you think
Raimar?

---
Vasco Alexandre da Silva Costa @ Instituto Superior Tecnico, Lisboa






[Prev in Thread] Current Thread [Next in Thread]