patchFreeciv - Patches: patch #1446, [Metaticket] Ruleset object

 
 
Show feedback again

patch #1446: [Metaticket] Ruleset object

Submitted by:  pepeto <pepeto>
Submitted on:  Sat 06 Feb 2010 01:22:07 PM UTC  
 
Category: rulesetsPriority: 1 - Later
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed 09 Jun 2010 09:55:28 PM UTC, comment #18:

I updated all all enum * patches; it seems to work but could somebody please check them; thanks!

[bug #1678] 01-trunk-add-SPECENUM_NUM
[bug #1646] 02-trunk-use-specenum_gen-for-enum-effect_type
[bug #1648] 03-trunk-use-specenum_gen-for-enum-req_range
[bug #1649] 04-trunk-use-specenum_gen-for-enum-tech_flag_id
[bug #1650] 05-trunk-use-specenum_gen-for-enum-impr_flag_id-and-enum-impr
[bug #1647] 06-trunk-use-specenum_gen-for-enum-universal_n
[bug #1658] 07-trunk-use-specenum_gen-for-enum-terrain_flag_id
[bug #1659] 08-trunk-use-specenum_gen-for-enum-terrain_class-and-enum-ter
[bug #1660] 09-trunk-use-specenum_gen-for-enum-mapgen_terrain_property
[bug #1661] 10-trunk-use-specenum_gen-for-enum-unit_class_flag_id

Matthias Pfafferodt <syntron>
Project Member
Sun 25 Apr 2010 05:20:12 PM UTC, comment #17:

> I am not sure, but maybe patch #1646 should use the names that are used in the ruleset file, maybe defining SPECENUM_VALUEx_NAMEs?


This is a good idea. It would move the definitions (enum and strings) to one place.

working version of file #9073 attached

(file #9074)

Matthias Pfafferodt <syntron>
Project Member
Sun 25 Apr 2010 05:01:28 PM UTC, comment #16:

I am not sure, but maybe patch #1646 should use the names that are used in the ruleset file, maybe defining SPECENUM_VALUEx_NAMEs?

pepeto <pepeto>
Project Member
Sun 25 Apr 2010 04:53:23 PM UTC, comment #15:

I tried to do it in one big step ... but this failed. The attached file includes my idea for the advances. I tried to do no changes to the main codebase (thus the file temp.(c|h)). The patch includes patch #1646 to patch #1652.

My new approach will be smaller: use the defined structs to create a secfile (load/save). The next steps will be the conversion into a (long) string (secfile_to_string()); send it to the client (one big network paket); parse it (secfile_from_str())and restore the ruleset file.

One big advantage is, that the ruleset will be independent from the network protocol. The big disadvantage is, that it needs a lot of time to get it right.

(file #9073)

Matthias Pfafferodt <syntron>
Project Member
Mon 19 Apr 2010 09:50:16 PM UTC, comment #14:

file #9018 is just a test patch, it's not for commit, it is not good at all... And yes, I made patch #1642 from a part of this test patch...

pepeto <pepeto>
Project Member
Mon 19 Apr 2010 09:43:38 PM UTC, comment #13:

> I did it, results without compression are :


I tried to test this using patch file #9018 but it seems to be missing some parts and contains most of patch #1642

Matthias Pfafferodt <syntron>
Project Member
Mon 19 Apr 2010 04:37:01 PM UTC, comment #12:

> Can you make in your side a estimation of the size of the packet you request?


I will try to setup the ruleset object and the corresponding secfile.

> Removing the nation file:


I think, that the nations should be separated from the rulesets. The rulesets only need a list of nations which are valid within the scope of the ruleset (or 'all').

I do plan to only include the nations of the players into the ruleset object which is saved into the savefile. The client needs only (?) the definition for the nation of the player.

Matthias Pfafferodt <syntron>
Project Member
Mon 19 Apr 2010 03:23:15 PM UTC, comment #11:

[1] I changed my mind, this looks an excellent idea after some checking.

pepeto <pepeto>
Project Member
Mon 19 Apr 2010 01:15:44 PM UTC, comment #10:

> Can you make in your side a estimation of the size of the packet
> you request?


I did it, results without compression are :
Very big!

With gzip compression (level 6):
It seems similar to send via binary packets then.

Removing the nation file:

  • Without compression: 137555 bytes.
  • With compression: 27273 bytes!

However, if I remember correctly the datas in the packets are compressed in common/packet.c? So it's not sure the compressed statistics are really usable...

(file #9018)

pepeto <pepeto>
Project Member
Mon 19 Apr 2010 12:25:39 PM UTC, comment #9:

New test for PACKET_RULESET_NATION stats (still default ruleset in trunk at revision 17383): those packets represents 79570 bytes, so more than the half of the whole stuff (only 56709 byes for the rest of the ruleset)...

It's quite a waste because a game cannot use more than 32 of them.
Maybe should we think about a solution where nations are not a part of the rulesets. Then, we could probably economize sending the rulesets when a connection is established. And maybe the client should send its own nation (maybe including the flag)? Then only used nation would be sent through the network?

pepeto <pepeto>
Project Member
Mon 19 Apr 2010 12:17:53 PM UTC, comment #8:

Is this much traffic? On-line games impossible? I do not have any data to compare. That is the size of the all packages currently needed to transfer the ruleset?

For information: I got the result of 136279 bytes received for the default ruleset in trunk at revision 17383 with the following hack in common/packets_gen.c:

Can you make in your side a estimation of the size of the packet you request?

I attached also the whole output. Note that the nation packets are very numerous and big!

> Also, the ruleset has to be transferred only once at the start
> of the game (or for longturn: start of the turn).


It is a very old behaviour of Freeciv version < 2.1. Now, the rulesets are sent every time a connection is established, notably for selecting your race at pre-game state. And if someone lose connection, it needs to receive again all the datas, which can make the game very laggy sometimes.

(file #9017)

pepeto <pepeto>
Project Member
Mon 19 Apr 2010 07:16:51 AM UTC, comment #7:

> [1] Do we need the client parse the ruleset file? I guess it would make a very big network traffic when someone will connect. It would probably make on-line games impossible, what do you think?


I don't know ... If I consider the default ruleset all definition files have a size of around 225K. Stripping all comments and additional spaces reduce it to 150K. Now, most data can be packed into tables (like all advances). But, on the other hand, to use a table all 'default' values have to be included. Thus, at the end around 150K must be transferred to the client.

Is this much traffic? On-line games impossible? I do not have any data to compare. That is the size of the all packages currently needed to transfer the ruleset? Also, the ruleset has to be transferred only once at the start of the game (or for longturn: start of the turn).

Furthermore, the ruleset data can be split into two parts:

  • for_server: only needed in the server; not transferred but has to be saved in a savegame
  • for_game: recalculated each time; transferred to the client but not saved (i.e. cost for advances, ...)
  • nations are not included in this ruleset! Only the names of the nations available for this ruleset are saved. The data is send separately to the client.
Matthias Pfafferodt <syntron>
Project Member
Sun 18 Apr 2010 09:10:05 PM UTC, comment #6:

[1] Do we need the client parse the ruleset file? I guess it would make a very big network traffic when someone will connect. It would probably make on-line games impossible, what do you think?

pepeto <pepeto>
Project Member
Sun 18 Apr 2010 08:39:51 PM UTC, comment #5:

Another idea:

use a ruleset file (see point 4 of comment 1) as 'packet' to transport all information to the client. This could be done as follows:

  • read the current rulesetdir into a ruleset object
  • verify the ruleset
  • transform it to a secfile (struct section_file)
  • use a new function char *secfile_as_string() to get a representation of the data as one string [1]
  • transfer this string (compressed; perhaps more than one packet) to the client(s)
  • verify the ruleset
  • restore the ruleset [2]

Part [1]/[2] can also be used to save/load the ruleset from a savegame. Perhaps all rulesetdirs should be converted into a ./data/*.ruleset file (*one* file with the entire ruleset!) at compilation time, i.e only this files have to be loaded. For ruleset developers the rulesetdir command would still allow to load a ruleset directory but a (new) freeciv-convert tool can be used to transform rulesetdirs into a ruleset file.

Matthias Pfafferodt <syntron>
Project Member
Sat 10 Apr 2010 10:47:17 AM UTC, comment #4:

> 1 & 2) I have already thought about a such solution for numerous structures of Freeciv. However, this is quite problematic. Most ruleset types for example need a translation support that need a gettext() pointer, which cannot be used in a packet structure.


What do you mean with gettext point? The for translation marked texts in the ruleset files? Internally the untranslated texts are used and they are translated before they are displayed, or?

Matthias Pfafferodt <syntron>
Project Member
Tue 06 Apr 2010 09:45:45 PM UTC, comment #3:

3) I agree.

4) I had similar idea. This would also give a way to save a ruleset into a savegame, and probably also to make a ruleset editor in a very far future...

pepeto <pepeto>
Project Member
Tue 06 Apr 2010 09:43:27 PM UTC, comment #2:

1 & 2) I have already thought about a such solution for numerous structures of Freeciv. However, this is quite problematic. Most ruleset types for example need a translation support that need a gettext() pointer, which cannot be used in a packet structure.

The delta code actually has some disadvantages that could be solved with lot of works. Actually, it doesn't send the packet structure through the network. It checks all values computed in it and send field per field and ignores changed fields. So, probably the packet structure could be avoided for sending. We could use accessors instead (like inline functions to read a field of a structure).

In receiving side, it is quite worse. For example, a client receiving a packet_city_info that only 1 field changed and was actually received, checks for all fields to make updates (in client/packhand.c). This make the client quite slow, especially with unit moves. A similar solution could be done, dropping the packet concept. common/packets_gen.c could call a function for each changed field that would probably save lot of time.

This is of course not a part of this item, unfortunately...

pepeto <pepeto>
Project Member
Tue 06 Apr 2010 09:16:22 PM UTC, comment #1:

some ideas about the ruleset rework:

1) at the moment all ruleset data used in the ruleset files is saved in the packet_ruleset_* structs; so use them for a ruleset object:

2) simplify the transfer of the ruleset; the available structs can be used; use the new ruleset struct everythere (if possible)

3) split the current ruleset file in two parts (and use code from the ruleset check on client side):
* load the data into the new ruleset object (loading the ruleset)
* verify the data (loading the ruleset and verifying the transferred ruleset on client side)

4) write a simple function which transforms the ruleset struct into a secfile; perhaps this could be done in generate_packets.py (save/write)? Only the packet_ruleset_* objects and the enums / values for the bv_* variables has to be saved

This way the ruleset directory can easily transformed into one (not easy to read?) ruleset file and the ruleset can also be saved in the savegame file.

With regard to the nations save only the names and load only the nations which are available and on the list.

What do you think about this?

Matthias Pfafferodt <syntron>
Project Member
Sat 06 Feb 2010 01:22:07 PM UTC, original submission:
pepeto <pepeto>
Project Member

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #9074:  ruleset.patch added by syntron (101kB - text/x-diff)
file #9073:  ruleset.patch added by syntron (100kB - text/x-diff)
file #9018:  RULESET_file_stats.diff added by pepeto (3kB - text/x-diff)
file #9017:  RULESET_receive_stats added by pepeto (89kB - application/octet-stream)

 

Digest:
   bug dependencies, patch dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Updated the item)
  • -unavailable- added by syntron (Posted a comment)
  • -unavailable- added by pepeto (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 25 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 18 Jun 2012 01:18:32 AM UTCjtnPlanned Release2.4.0=>
    Wed 27 Oct 2010 01:53:49 PM UTCpepetoPlanned Release2.3.0=>2.4.0
    Mon 05 Jul 2010 10:17:38 PM UTCsyntronDependencies-=>Depends on patch #1735
    Thu 17 Jun 2010 11:02:02 AM UTCsyntronDependencies-=>Depends on patch #1338
    Fri 21 May 2010 04:28:21 PM UTCsyntronDependencies-=>Depends on patch #1678
    Sun 02 May 2010 09:45:30 PM UTCsyntronDependencies-=>Depends on patch #1662
    Sun 02 May 2010 09:37:03 PM UTCsyntronDependencies-=>Depends on patch #1661
      Dependencies-=>Depends on patch #1660
      Dependencies-=>Depends on patch #1659
      Dependencies-=>Depends on patch #1658
    Sun 25 Apr 2010 05:20:12 PM UTCsyntronAttached File-=>Added ruleset.patch, #9074
    Sun 25 Apr 2010 04:53:23 PM UTCsyntronAttached File-=>Added ruleset.patch, #9073
    Sun 25 Apr 2010 04:43:27 PM UTCsyntronDependencies-=>Depends on patch #1651
    Sun 25 Apr 2010 04:43:05 PM UTCsyntronDependencies-=>Depends on patch #1652
      Dependencies-=>Depends on patch #1650
      Dependencies-=>Depends on patch #1649
      Dependencies-=>Depends on patch #1648
      Dependencies-=>Depends on patch #1647
      Dependencies-=>Depends on patch #1646
    Mon 19 Apr 2010 01:15:44 PM UTCpepetoAttached File-=>Added RULESET_file_stats.diff, #9018
    Mon 19 Apr 2010 12:17:53 PM UTCpepetoAttached File-=>Added RULESET_receive_stats, #9017
    Sun 07 Feb 2010 11:30:49 AM UTCpepetoDependencies-=>Depends on bugs #13873
    Sat 06 Feb 2010 01:29:21 PM UTCpepetoDependencies-=>Depends on patch #1449
      Dependencies-=>Depends on patch #1448
      Dependencies-=>Depends on patch #1447
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup