@cazfi. I will look if it is real necessary, but I probably allocates counters inside routine creating city (both on client and server). As far, as I remember, default value of counter (not initial for city) was send in ruleset packet.
Reply To lachu
@cazfi. I will look if it is real necessary, but I probably allocates counters inside routine creating city (both on client and server). As far, as I remember, default value of counter (not initial for city) was send in ruleset packet.
Yes, memory is allocated when the city is created, and the client knows the default counter value. What it currently does not know after connecting to the server is the counter's current value, until there's a change to it.
Reply To cazfi
Reply To lachu
@cazfi. I will look if it is real necessary, but I probably allocates counters inside routine creating city (both on client and server). As far, as I remember, default value of counter (not initial for city) was send in ruleset packet.
Yes, memory is allocated when the city is created, and the client knows the default counter value. What it currently does not know after connecting to the server is the counter's current value, until there's a change to it.
Ah, yes - client can connect in middle of the game or server could load savegame. Thanks for clarification.
Reply To cazfi
Reply To lachu
@cazfi. I will look if it is real necessary, but I probably allocates counters inside routine creating city (both on client and server). As far, as I remember, default value of counter (not initial for city) was send in ruleset packet.
Yes, memory is allocated when the city is created, and the client knows the default counter value. What it currently does not know after connecting to the server is the counter's current value, until there's a change to it.
Done and tested: firstly wrote the patch and test - reconnects and load savegame. In next step, do git format-patch, reset ==hard HEAD~1 and retest.
I think that information should be sent always when the full city packet gets sent for the first time, not only when the client reconnects. I don't think the current implementation works, e.g., when one conquers an enemy city (well, we only have the "owned" counter at the moment, and that's zeroed at conquest anyway, but in the general case the new owner should get correct counter values). Correct me if that gets handled somehow.
Reply To cazfi
I think that information should be sent always when the full city packet gets sent for the first time, not only when the client reconnects. I don't think the current implementation works, e.g., when one conquers an enemy city (well, we only have the "owned" counter at the moment, and that's zeroed at conquest anyway, but in the general case the new owner should get correct counter values). Correct me if that gets handled somehow.
I will look inside srv_main.c, but I remember there exist check counter was changed and it will be resend in this case. That mean, if city get conquered, new value will be send to the new owner, but it is handled only on turn-player-switch. So, I must repair current code - you're right.
Reply To cazfi
I think that information should be sent always when the full city packet gets sent for the first time, not only when the client reconnects. I don't think the current implementation works, e.g., when one conquers an enemy city (well, we only have the "owned" counter at the moment, and that's zeroed at conquest anyway, but in the general case the new owner should get correct counter values). Correct me if that gets handled somehow.
You are right. in srv_main.c we send only value, when counter change, so on end of each turn. We must also send value to new owner (and observers), when city owner's changed.
Reply To cazfi
I think that information should be sent always when the full city packet gets sent for the first time, not only when the client reconnects. I don't think the current implementation works, e.g., when one conquers an enemy city (well, we only have the "owned" counter at the moment, and that's zeroed at conquest anyway, but in the general case the new owner should get correct counter values). Correct me if that gets handled somehow.
I messed up. I look at 4cab5d9a0e2fa958213fe3060569b4efec2dc55d and see this Ticket is implemented by it. Does not it?
Reply To lachu
I messed up. I look at 4cab5d9a0e2fa958213fe3060569b4efec2dc55d and see this Ticket is implemented by it. Does not it?
Is the code in there for the generic case? I have to recheck.
Reply To cazfi
Reply To lachu
I messed up. I look at 4cab5d9a0e2fa958213fe3060569b4efec2dc55d and see this Ticket is implemented by it. Does not it?
Is the code in there for the generic case? I have to recheck.
Not for any kind of counter, but for owned and possibly other already implemented counter. Generic case is hard to handle, because when to send counter of any type? In this thread you suggest, owned should been send on sending city information. Should I send each counter on turn end also@
Reply To lachu
should been send on sending city information.
That just seems obvious to me - city counter values *are* part of city related information, aren't they?
As for trying to figure out what counters to send on each situation, on some high level functions; 1) delta network protocol already automatically makes it so that not-changed values are not actually sent. 2) Implementing it on high level will only bring very complex code, with lots of bugs, high maintenance, and all the special case handling code making bigger NEGATIVE performance impact than what one would save in avoiding refreshing some field.
Reply To cazfi
Reply To lachu
should been send on sending city information.
That just seems obvious to me - city counter values *are* part of city related information, aren't they? As for trying to figure out what counters to send on each situation, on some high level functions; 1) delta network protocol already automatically makes it so that not-changed values are not actually sent. 2) Implementing it on high level will only bring very complex code, with lots of bugs, high maintenance, and all the special case handling code making bigger NEGATIVE performance impact than what one would save in avoiding refreshing some field.
I think I do not fully understood you. I wonder what means send counter information, when client disconnects. In this thread you wrote, that better place for owned/celebrated/disorder counter should be send with rest of city information. Currently, we supports only these three type of counters. My question is: for what this ticket exists? Do I implement this ticket in patches applied from other ticket?
Reply To lachu
In this thread you wrote, that better place for owned/celebrated/disorder counter should be send with rest of city information. Currently, we supports only these three type of counters.
We want to build support for further counters, not just those three.
Reply To cazfi
Reply To lachu
In this thread you wrote, that better place for owned/celebrated/disorder counter should be send with rest of city information. Currently, we supports only these three type of counters.
We want to build support for further counters, not just those three.
Hi. How does delta protocol recognized similar packed. I mean, there is need to select key field, so changes in key field do not mess up. For example - I send PLAYER_ONE COUNTER_0 COUNTER_VALUE=50 and next PLAYER_TWO COUNTER_0 COUNTER_VALUE=100 and lastly PLAYER_ONE COUNTER_0 COUNTER_VALUE=100 . First message should be removed from memory and do not compared again, so when I try to send PLAYER_ONE COUNTER_0 COUNTER_VALUE=50, it was send.
Ok. Seems that counters network protocol isn't in the shape I though it would be. Will open a new ticket about reworking that. Shouldn't block this ticket, though (just means that the protocol isn't as efficient as it should)
Reply To cazfi
Ok. Seems that counters network protocol isn't in the shape I though it would be. Will open a new ticket about reworking that. Shouldn't block this ticket, though (just means that the protocol isn't as efficient as it should)
Should I still use delta protocol? How it works?
Reply To lachu
Should I still use delta protocol? How it works?
It's a low level protocol in the network packet management code. You've been using it all the time.
Reply To cazfi
Reply To lachu
Should I still use delta protocol? How it works?
It's a low level protocol in the network packet management code. You've been using it all the time.
I put some little changes into https://osdn.net/projects/freeciv/ticket/45890 . I do not known, where to put them - no ticket opened, but it seems changes are related, but topic do not describe changes in last two patch.
Reply To cazfi
Reply To lachu
Should I still use delta protocol? How it works?
It's a low level protocol in the network packet management code. You've been using it all the time.
I tested current implementation (without changes by patches suggested in this ticket) and it seems to working actually, probably because server send city info, when client request. I test celebration effect and additional unhappiness effect for 5-7 turns once conquering city. I will also test restriction to producing warriors once conquering city. I think I need some case, where client must calculates data. I figure out (last mind), what happens if I do not open city window and state of city should change, but I must known what to test. Gold probably is bad idea. Science points is probably also bad idea. I must figure out.
Reply To cazfi
Reply To lachu
Should I still use delta protocol? How it works?
It's a low level protocol in the network packet management code. You've been using it all the time.
I look inside connecthand and there's no word city, so any city_itterate is also missing. But player must known, if city is disorder (icon), so I probably must search,
Reply To cazfi
I think that information should be sent always when the full city packet gets sent for the first time, not only when the client reconnects. I don't think the current implementation works, e.g., when one conquers an enemy city (well, we only have the "owned" counter at the moment, and that's zeroed at conquest anyway, but in the general case the new owner should get correct counter values). Correct me if that gets handled somehow.
About sending new values, I think send_city_info do the trick.
Reply To cazfi
I think that information should be sent always when the full city packet gets sent for the first time, not only when the client reconnects. I don't think the current implementation works, e.g., when one conquers an enemy city (well, we only have the "owned" counter at the moment, and that's zeroed at conquest anyway, but in the general case the new owner should get correct counter values). Correct me if that gets handled somehow.
2023-03-04 23:42 Updated by: lachu
2023-03-04 23:42 Updated by: lachu
Probably works, but something is odd. After conquering, the values are bigger than normal, but reduced (I restore population). Not each citizen are happy, so maybe this is a reason? I do:
commit 9b1937aa1fba3bb37ef0a25e2409e195363dd54e (HEAD -> MAIN/WIP/COUNTERS/PACKHAND/RESEND_VALUES) Author: Sławomir Lach <slawek@lach.art.pl> Date: Sat Mar 4 14:29:01 2023 +0100
commit 80a59e4d7efd8c99d884d25546ccfb53da99df88 Author: Sławomir Lach <slawek@lach.art.pl> Date: Sat Mar 4 14:01:58 2023 +0100
commit 2340e9cc2dc409b5169746163fcdab0ee637f8a5 Author: Sławomir Lach <slawek@lach.art.pl> Date: Sat Mar 4 14:00:12 2023 +0100
commit c7e10401448d4891dddc323ed18ae835bc2cec44 Author: Sławomir Lach <slawek@lach.art.pl> Date: Sat Mar 4 13:59:48 2023 +0100
And base is:
So maybe it is working already?
But on main branch, the same effect (lower than excepted values after conquering).
EDIT: Maybe it is not send, because delta protocol do not see changes. Our packet is defined as follows:
There is no player-key field. I try to look at this tomorrow.
Is there any way to invalidate delta packet (by packet type and keys)?
Reply To cazfi
City counter value updates are sent to client by #45429, but we will need to send also initial values, e.g., when a client reconnects, or game has been loaded from a saved game. Basically when ever full city info gets sent to the client for the first time (note, that if it makes implementation easier; it should be ok to "send" them to the delta protocol layer unconditionally when ever full city info gets sent. Delta protocol would notice itself if there's nothing to update/send)
It worked without patch from this ticket. Only apply this to test. I:
1. Create 10x10 land area
2. Create city on this area and near enemy warrior/unit
3. Set city size to 10 and foodstock to 109
4. Wait 3 turns - production explodes!
5. Conquer city - production still is big
I wonder, what should be done if I conquer disordering city. Should I reset disorder counter? I only test celebration counter, but other should also works.
Reply To lachu
5. Conquer city - production still is big
As calculated by the client, or just by the server side?
Reply To cazfi
Reply To lachu
5. Conquer city - production still is big
As calculated by the client, or just by the server side?
I do not known. Firstly, the city owner is changed and the new owner do not have counter's values information before conquering the city. Server must send a new data and new values are still taking celebrating turn count (because counter is set). Ok. Maybe I not be right. It works, but I do not known if server sends food, production, trade, etc., or if client calculates this based on counter's value. I think, there could be need to create counters' values tab on city window. I will try to use gdb on client process to figure out, what really happens.
Reply To cazfi
Reply To lachu
5. Conquer city - production still is big
As calculated by the client, or just by the server side?
I did not worked. I do not known, why. I must investigate. It seems client do not receive counter value. I do not conquer city and play only few turns:
GDB:
Part of savegame:
Reply To cazfi
Reply To lachu
5. Conquer city - production still is big
As calculated by the client, or just by the server side?
Above bug was repaired and new values are send once game loaded, but not when reconnected.
I do not known if my changes (in packhand.c, routine responsible for handling packets related to ruleset-orinted counter's data) could be pasted here. Maybe create new ticket, to put patch there? Change was simple - just add additional condition checking if value is not another value of counter type, so we handles also CELEBRATING and DISORDER counters.
Inside void handle_ruleset_counter(const struct packet_ruleset_counter *packet) we return if counter behavior is not CITY_OWNED_TURNS. I change this if statement.
Reply To cazfi
Reply To lachu
5. Conquer city - production still is big
As calculated by the client, or just by the server side?
2023-03-13 01:59 Updated by: lachu
2023-03-13 01:59 Updated by: lachu
(File ID: 11834): It is bug fix, which is not related to this ticket, but necessary to made ruleset data handling correct. I put it here to not waste time.
(File ID: 11835): send_city_info is not always called, when city info was send - for example - workers rearrangement. Now, I send data in two functions called by send_city_info. There is no case, where data was send twice, because (at least) send_city_info calls only one of these routines in single invocation of it. I must check, if there is another case, where both functions (broadcast_city_info, send_city_info_at_tile) were called.
Reply To lachu
which is not related to this ticket
Err.. is some patch here one that should solve this ticket?
Reply To cazfi
Reply To lachu
@cazfi. I will look if it is real necessary, but I probably allocates counters inside routine creating city (both on client and server). As far, as I remember, default value of counter (not initial for city) was send in ruleset packet.
Yes, memory is allocated when the city is created, and the client knows the default counter value. What it currently does not know after connecting to the server is the counter's current value, until there's a change to it.
Hi. I do not read whole ticket. I see (on main), when city was built, counter values are send to client. Should I wrote some more generic approach? Current only sends city counter's value, but packet definition are tied to cities. I can write generic code, but we must take care about entity kind city/unit/player/world inside procedure sending these values, so it does not makes sense (in my opinion).
Reply To cazfi
Reply To lachu
which is not related to this ticket
Err.. is some patch here one that should solve this ticket?
I use git branch --contains commit_id which commit id gathered via git log. Everything is in place currently. Just response if you need more generic way.
EDIT: There you are clarification was goes wrong and why I messed up in this ticket. I was fought with bug described in this post: 2023-03-13 01:46 Updated by: lachu . But, as I wrote, every patch, with should makes everything worked, was applied to main.
Reply To lachu
Reply To cazfi
Reply To lachu
which is not related to this ticket
Err.. is some patch here one that should solve this ticket?
I use git branch --contains commit_id which commit id gathered via git log. Everything is in place currently. Just response if you need more generic way. EDIT: There you are clarification was goes wrong and why I messed up in this ticket. I was fought with bug described in this post: 2023-03-13 01:46 Updated by: lachu . But, as I wrote, every patch, with should makes everything worked, was applied to main.
There's nothing pushed in addressing this issue, and it's easy to produce a failing case.
- Add these to civ2civ3:
Counter to game.ruleset
[counter_owned] name = "Owned 5 Turns" checkpoint = 5 def = 0 type = "Owned"
Effect to effects.ruleset
[effect_owned_bonus]
type = "Output_Bonus"
value = 1000
reqs =
{ "type", "name", "range"
"Counter", "Owned 5 Turns", "City"
"OutputType", "Gold", "Local"
}
- Launch server and client separately
- Connect client to the server
- Found a city, and press "Turn Done" five times
- Observe on the client how the city is now getting 1000% gold bonus, and majority of it is because of the counter (left mouse button over the "Gold" amount to get the breakdown)
- Disconnect and Reconnect the client
- Observe how majority of the city's Gold income is now from a source "unknown" to the client (total sum is correct, as server does send that information)
Reply To cazfi
Reply To lachu
Reply To cazfi
Reply To lachu
which is not related to this ticket
Err.. is some patch here one that should solve this ticket?
I use git branch --contains commit_id which commit id gathered via git log. Everything is in place currently. Just response if you need more generic way. EDIT: There you are clarification was goes wrong and why I messed up in this ticket. I was fought with bug described in this post: 2023-03-13 01:46 Updated by: lachu . But, as I wrote, every patch, with should makes everything worked, was applied to main.
There's nothing pushed in addressing this issue, and it's easy to produce a failing case. ---- - Add these to civ2civ3:
Counter to game.ruleset {{{ counter_owned name = "Owned 5 Turns" checkpoint = 5 def = 0 type = "Owned" }}} Effect to effects.ruleset {{{ effect_owned_bonus type = "Output_Bonus" value = 1000 reqs = { "type", "name", "range" "Counter", "Owned 5 Turns", "City" "OutputType", "Gold", "Local" } }}} - Launch server and client separately
- Connect client to the server
- Found a city, and press "Turn Done" five times
- Observe on the client how the city is now getting 1000% gold bonus, and majority of it is because of the counter (left mouse button over the "Gold" amount to get the breakdown)
- Disconnect and Reconnect the client
- Observe how majority of the city's Gold income is now from a source "unknown" to the client (total sum is correct, as server does send that information)
2023-07-04 17:15 Updated by: lachu
Sorry it takes too long.
Reply To cazfi
Hi. Sorry for asks. Did you miss notification? I send patches to two tickets few weeks ago.
The place where you put the call seems rather arbitrary - something constantly called outside connection establishing case, and I'm not sure it's called during that establishing (except by the luck of getting called so often). I would imagine you would need to touch:
/**********************************************************************//** Send all information for when game starts or client reconnects. Initial packets should have been sent before calling this function. See comment in connecthand.c::establish_new_connection(). **************************************************************************/ void send_all_info(struct conn_list *dest)
Reply To cazfi
The place where you put the call seems rather arbitrary - something constantly called outside connection establishing case, and I'm not sure it's called during that establishing (except by the luck of getting called so often). I would imagine you would need to touch: {{{ /**********************************************************************//** Send all information for when game starts or client reconnects. Initial packets should have been sent before calling this function. See comment in connecthand.c::establish_new_connection(). **************************************************************************/ void send_all_info(struct conn_list *dest) }}}
I remember that send_all_info calls in chain routine in which I insert call to update city counters routine. I will check this. If send_all_info call routine I modify, than there was more chance to my code being executed.
Reply To cazfi
The place where you put the call seems rather arbitrary - something constantly called outside connection establishing case, and I'm not sure it's called during that establishing (except by the luck of getting called so often). I would imagine you would need to touch: {{{ /**********************************************************************//** Send all information for when game starts or client reconnects. Initial packets should have been sent before calling this function. See comment in connecthand.c::establish_new_connection(). **************************************************************************/ void send_all_info(struct conn_list *dest) }}}
Content distribution network of OSDN still did not work, so I put many copy of the same files by accident.
I don't think you should be excluding observers from receiving the information. They should be able to observe it.
Likely something like this pseudocode:
players_iterate() {
cities_iterate(player) {
conn_iterate(player connections) {
send_city_counters()
} conn_iterate_end;
conn_iterate(global observer connections) {
send_city_counters();
} conn_iterate_end;
} cities_iterate_end;
} players_iterate_end;
Reply To cazfi
Likely something like this pseudocode: {{{ players_iterate() { cities_iterate(player) { conn_iterate(player connections) { send_city_counters() } conn_iterate_end; conn_iterate(global observer connections) { send_city_counters(); } conn_iterate_end; } cities_iterate_end; } players_iterate_end; }}}
Look at server/city_turn.c : city_counters_refresh
We send each counter value to each global observer.
It is simple fix. I will also follow path you suggested and do more complicated piece of code, but simple fix I send just works.
More complicated implementation. By functional, simplest and this are the same.No differences, but " 0001-OSDN-45889-Complicated-implementation.patch(1KB)" have more code. In fact, I have only to copy code from one runtime.
With what you told, "simple fix" should be enough -> it's the one I plan to push.
City counter value updates are sent to client by #45429, but we will need to send also initial values, e.g., when a client reconnects, or game has been loaded from a saved game. Basically when ever full city info gets sent to the client for the first time (note, that if it makes implementation easier; it should be ok to "send" them to the delta protocol layer unconditionally when ever full city info gets sent. Delta protocol would notice itself if there's nothing to update/send)