@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.
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)