Ticket #47697

Counter description

Eröffnet am: 2023-03-26 19:49 Letztes Update: 2023-05-01 11:11

Auswertung:
Verantwortlicher:
Typ:
Status:
Geschlossen
Komponente:
Meilenstein:
Priorität:
5 - Mittel
Schweregrad:
5 - Mittel
Lösung:
Gefixt
Datei:
10

Details

If we're going to show known counter values to the user on the client side, we should provide also longer explanation for them instead of just name.

This ticket is about adding such explanation to the ruleset format, and communicating it to client.

Ticket-Verlauf (3/31 Historien)

2023-03-26 19:49 Aktualisiert von: cazfi
  • New Ticket "Counter description" created
2023-03-26 22:23 Aktualisiert von: lachu
Kommentar

I think we should move checkpoint onto separated entity, called trigger, which will contains counter, description, min and max fields. It would allow to do more flexible thinks. Checkpoint reqs will be related to trigger, not counter. It will allow us to be more flexible. Checkpoint will be fulfilled, when value of counter is inside min and max.

2023-03-27 09:09 Aktualisiert von: cazfi
Kommentar

Reply To lachu

I think we should move checkpoint onto separated entity, called trigger, which will contains counter, description, min and max fields. It would allow to do more flexible thinks. Checkpoint reqs will be related to trigger, not counter. It will allow us to be more flexible. Checkpoint will be fulfilled, when value of counter is inside min and max.

And to do all that still within 3.2 cycle, instead of just adding the description field as I proposed?

2023-03-31 02:11 Aktualisiert von: lachu
Kommentar

Reply To cazfi

If we're going to show known counter values to the user on the client side, we should provide also longer explanation for them instead of just name. This ticket is about adding such explanation to the ruleset format, and communicating it to client.

0001-OSDN-47697-S-awomir-Lach-slawek-lach.art.pl.patch(2KB)
Ruleset changes to test modifications

0002-OSDN-TICKET-47697-S-awomir-Lach-s-awek-lach.art.pl.patch(2KB)

Server/ruleset-related changes

0003-OSDN-TICKET-47697-S-awomir-Lach-slawek-lach.art.pl.patch(3KB)

Client-side changes

I think second and third could be applied.

2023-04-03 02:03 Aktualisiert von: cazfi
Kommentar

Sorry it takes so long for me to get to these. Upcoming milestones from older branches keep me quite busy.

I had a bit different thing in my mind, but going to help system already is fine too. (I was thinking something like city dialog listing current counter values and a couple of words description what those values mean)

Also splitting this to two patches probably makes merging easier (one part at a time), so let's concentrate on the ruleset/server part here (0002... patch), and create a new ticket about the client / help system changes (0003... patch)

---

- The server side patch seems to be missing rulesave part
- Documentation comments in rulesets and data/ruledit/comments-3.x.txt missing
- I guess loading side should not make the help text mandatory (or if it does, then it should be added to all rulesets)

2023-04-08 19:21 Aktualisiert von: cazfi
Kommentar

Reply To cazfi

create a new ticket about the client / help system changes (0003... patch)

-> #47804

2023-04-10 18:20 Aktualisiert von: lachu
Kommentar

Reply To cazfi

Reply To cazfi

create a new ticket about the client / help system changes (0003... patch)

-> #47804

Reply To cazfi

Sorry it takes so long for me to get to these. Upcoming milestones from older branches keep me quite busy. I had a bit different thing in my mind, but going to help system already is fine too. (I was thinking something like city dialog listing current counter values and a couple of words description what those values mean) Also splitting this to two patches probably makes merging easier (one part at a time), so let's concentrate on the ruleset/server part here (0002... patch), and create a new ticket about the client / help system changes (0003... patch) --- - The server side patch seems to be missing rulesave part
- Documentation comments in rulesets and data/ruledit/comments-3.x.txt missing
- I guess loading side should not make the help text mandatory (or if it does, then it should be added to all rulesets)

0001-OSDN-47697-S-awomir-Lach-slawek-lach.art.pl.patch(2KB)

Changes in ruleset handling and network

0001-OSDN-47697-S-awomir-Lach-slawek-lach.art.pl.patch(9KB)

Documentation
2023-04-14 21:41 Aktualisiert von: lachu
Kommentar

Reply To cazfi

Sorry it takes so long for me to get to these. Upcoming milestones from older branches keep me quite busy. I had a bit different thing in my mind, but going to help system already is fine too. (I was thinking something like city dialog listing current counter values and a couple of words description what those values mean) Also splitting this to two patches probably makes merging easier (one part at a time), so let's concentrate on the ruleset/server part here (0002... patch), and create a new ticket about the client / help system changes (0003... patch) --- - The server side patch seems to be missing rulesave part
- Documentation comments in rulesets and data/ruledit/comments-3.x.txt missing
- I guess loading side should not make the help text mandatory (or if it does, then it should be added to all rulesets)

You are missing part of city dialog. If this ticket should only contains server-side patches, we lost track about client-side/city-dialog changes.

2023-04-20 21:43 Aktualisiert von: lachu
Kommentar

I add new ticket: #47887 for client-side (already only gtk3.22 client) implementation of checking counter value/ruleset-information for city.

2023-04-22 12:08 Aktualisiert von: cazfi
Kommentar

Reply To lachu

You are missing part of city dialog. If this ticket should only contains server-side patches, we lost track about client-side/city-dialog changes.

There was #47804 split earlier, about help system changes. But it's ok if #47887 is about city dialog changes. Those two are quite distinct parts of the client, so deserve separate tickets.

2023-04-22 12:16 Aktualisiert von: cazfi
Kommentar

- Seems to me that freeing (or destroying, bý strvec_destroy(), of the help texts is missing, as well as initializing them (these are related in that you don't know if non-NULL vector is to be destroyed or not, if it might be just uninitialized pointer)

2023-04-23 21:24 Aktualisiert von: lachu
Kommentar

Reply To cazfi

- Seems to me that freeing (or destroying, bý strvec_destroy(), of the help texts is missing, as well as initializing them (these are related in that you don't know if non-NULL vector is to be destroyed or not, if it might be just uninitialized pointer)

2023-04-23 21:24 Updated by: lachu

File 0001-OSDN-47697-S-awomir-Lach-slawek-lach.art.pl.patch (File ID: 12271) is attached
2023-04-24 07:07 Aktualisiert von: cazfi
Kommentar
+void counters_free(void);

It's already declared in counters.h.

---
 void counters_init(void)
 {
+  if (0 < game.control.num_counters) {
+    counters_free();
+  }

I'm not sure if game.control.num_counters is initialized at all when _init() gets called the very first time. Also, I think counters follow (or they should follow) the common pattern that there's always matching _free() call for each _init(), so this kind of code should never be needed. One could make it an assert, if one suspects that _free() might have not been called properly: "fc_assert(game.control.num_counters == 0);"

2023-04-27 03:23 Aktualisiert von: lachu
Kommentar

Reply To cazfi

{{{ +void counters_free(void); }}} It's already declared in counters.h. --- {{{ void counters_init(void) { + if (0 < game.control.num_counters) { + counters_free(); + } }}} I'm not sure if game.control.num_counters is initialized at all when _init() gets called the very first time. Also, I think counters follow (or they should follow) the common pattern that there's always matching _free() call for each _init(), so this kind of code should never be needed. One could make it an assert, if one suspects that _free() might have not been called properly: "fc_assert(game.control.num_counters == 0);"

0001-OSDN-47697-S-awomir-Lach-slawek-lach.art.pl.patch(3KB)
Repair bug in rulesave (use bad function) and changes in intiialization/deinitialization routines.
2023-04-27 04:02 Aktualisiert von: cazfi
Kommentar

The code seems fine now -> adding to autogame test runners already.

On the documentation part, I noticed that there's "\n\" in the end of lines also in the actual rulesets. Those are needed in comments.txt to generate ruleset files, but not in the final rulesets (they have actual newlines in place already). Sorry for not noticing this in the earlier review.

2023-04-27 10:42 Aktualisiert von: cazfi
Kommentar

Ruleup for S3_2 sandbox ruleset segfaults on main branch.

Program received signal SIGSEGV, Segmentation fault.
strvec_size (psv=0x0) at ../../../src/utility/string_vector.c:345
345	  return psv->size;
(gdb) bt
#0  strvec_size (psv=0x0) at ../../../src/utility/string_vector.c:345
#1  0x000055555587951e in save_game_ruleset (
    filename=filename@entry=0x7fffffffd030 "sandbox.ruleup/game.ruleset", 
    name=name@entry=0x555555bc3169 <game+233> "Sandbox ruleset")
    at ../../../../src/tools/ruleutil/rulesave.c:1815
#2  0x000055555587affb in save_ruleset (path=path@entry=0x7fffffffd680 "sandbox.ruleup", 
    name=0x555555bc3169 <game+233> "Sandbox ruleset", data=data@entry=0x7fffffffd670)
    at ../../../../src/tools/ruleutil/rulesave.c:3394
#3  0x000055555559b232 in main (argc=<optimized out>, argv=<optimized out>)
    at ../../../src/tools/ruleup.c:226
2023-04-27 22:20 Aktualisiert von: lachu
Kommentar

Reply To cazfi

Ruleup for S3_2 sandbox ruleset segfaults on main branch. {{{ Program received signal SIGSEGV, Segmentation fault. strvec_size (psv=0x0) at ../../../src/utility/string_vector.c:345 345 return psv->size; (gdb) bt #0 strvec_size (psv=0x0) at ../../../src/utility/string_vector.c:345 #1 0x000055555587951e in save_game_ruleset ( filename=filename@entry=0x7fffffffd030 "sandbox.ruleup/game.ruleset", name=name@entry=0x555555bc3169 <game+233> "Sandbox ruleset") at ../../../../src/tools/ruleutil/rulesave.c:1815 #2 0x000055555587affb in save_ruleset (path=path@entry=0x7fffffffd680 "sandbox.ruleup", name=0x555555bc3169 <game+233> "Sandbox ruleset", data=data@entry=0x7fffffffd670) at ../../../../src/tools/ruleutil/rulesave.c:3394 #3 0x000055555559b232 in main (argc=<optimized out>, argv=<optimized out>) at ../../../src/tools/ruleup.c:226 }}}

2023-04-27 22:19 Updated by: lachu

File 0001-OSDN-47697-S-awomir-Lach-slawek-lach.art.pl.patch (File ID: 12305) is attached

I now looks good. I do not known (really), how to test it. I do not known, which parameters do you use to invoke ruleup.

2023-04-27 22:28 Aktualisiert von: lachu
Kommentar

Reply To cazfi

The code seems fine now -> adding to autogame test runners already. On the documentation part, I noticed that there's "\n\" in the end of lines also in the actual rulesets. Those are needed in comments.txt to generate ruleset files, but not in the final rulesets (they have actual newlines in place already). Sorry for not noticing this in the earlier review.

0001-OSDN-47697-S-awomir-Lach-slawek-lach.art.pl.patch(9KB)
New version of documentation
2023-04-28 06:38 Aktualisiert von: cazfi
Kommentar

Reply To lachu

I do not known (really), how to test it. I do not known, which parameters do you use to invoke ruleup.

export FREECIV_DATA_PATH="/fast/freeciv/S3_2/src/data:/fast/freeciv/main/src/data"
./fcruleup -r sandbox
2023-04-28 14:56 Aktualisiert von: cazfi
  • Verantwortlicher Update from (Keine) to cazfi
  • Lösung Update from Keine to Accepted
  • Meilenstein Update from S3_2 npf to S3_2 d3f (closed)
2023-05-01 11:11 Aktualisiert von: cazfi
  • Status Update from Offen to Geschlossen
  • Lösung Update from Accepted to Gefixt

Dateianhangliste

Bearbeiten

Please login to add comment to this ticket » Anmelden