#601 hex groups
Creates 5 random hex groups. The hex groups show up on the map editor as a single colored hex on the map, depending on the hex group. A hex group page is added to allow changing the amount of terrains and chits to be assigned to that hex group. The page also allows for boolean options. When the map is randomized with the server, the hex groups are randomized using the the tiles and chits assigned to that hex group and options are applied to the hex group. Currently there are two options, fog and no-setup. Fog changes a fog variable added to each hex and does not yet do anything else. No-setup changes the adjacent nodes no-setup unless there is a neighboring hex that allows it.
Since I replaced the old randomize function, I depreciated the chits data in the map file and placed the roll info directly into the map array in the file. I also depreciated the '+' since all non hex group tiles are not randomized. If the chits data is used, it adds all non sea non '+' to hex group 0.
I since I made this patch using rev 1647, there is not currently a way to change hexes to a hex group using the editor. The only way is to modify the .game file directly. This functionality will be added later.
I have removed the initial robber placement. This should be changed back unless playing cities and knights. The greedy AI assumed the robber was always on a hex. I have fixed the AI.
I have changed default.game to use hex groups. Unlike the rest of the map files, there was no copyright info in default.game. Was this copyrighted before, if so we need to give proper credit.
Builds the hex_group page in editor. Needs to be in editor/gtk/
header file for respected .c file Needs to be in editor/gtk/
After creating the progress cards patch, and learning how the other settings are programmed, I decided that I should change this patch to use a composite widget to provide better encapsulation.
This is an old patch... before I knew how to svn add...
Steps to run:
svn up -r 1647
svn patch hex_group.patch
TODO:
Merge with current svn rev.
Add context support.
Add toolbar support (requires editor toolbars).
Review code(I have learned a lot since I made this).
I'm at the moment updating the patch to the current revision.
The drawing routines have been modified since the patch was written.
I already have some comments:
If you want to have an easier update to the current svn revision, run 'make reindent' after you've applied the patch. That will reduce some whitespace related issues. (I'm working on that)
Can you keep the protocol (storing/sending the map) as much backwards compatible as possible? (At the metaserver I see still many 0.12 clients)
Naming convention: use CamelCase for the datatypes, not a combination of C-style naming an CamelCase (Hex_Group -> HexGroup)
HexGroup->color: what kind of non-gui datamember is this?
Possible more...
I know there are a lot of hits... The review code was mainly for me.
I remembering the map using the old protocol after the map was randomized.
I will look at it after you have updated the patch.
For you possible more comments, don't bother with convention comments yet as I said earlier, I have learned a lot since I made this and need to review it. At this point general directional comments are more helpful.
Attached is the version of the patch updated to 1919.
I've tested it with the editor: './pioneers-editor server/default.game'
When run with other games, it will crash the editor.
I've fixed a bug in copy_hex, when the hex_group was NULL.
In the editor I've resized the chits to 30x30 instead of 24x24, they are now round.
The patch is up-to-date now, but can you describe first how it is supposed to work?
The way to create a board layout seems a bit complicated to me. Instead of drawing a board, I now must think in groups. How about a board that should not be shuffled. How many groups would I need?
I've removed the fix in client/ai/greedy.c, it was already fixed in another way.
About the drawing code:
Since the code is now using cairo, it mights even be possible to show the hex group colour with a transparency of e.g. 20%, so it might be possible to assign a hex to a group at any moment in the game, (And have the group be shown in the editor).
Hints for your review:
- Write a short document with the goals before continuing to code.
- Coding convension: C-style for variable and function names, CamelCase for types
- Check the spelling (also in comments) gruop->group
- I tink I've found an unused member in a struct
- Keep model and view separate: no color member is HexGroup
- Use autogen.sh or compile with at least '--enable-warnings'
- Try to find some way to be backwards compatible (at least back to 0.12)
This is a bug, all other games should run fine. Will look into.
When the board is not to be shuffled, groups would not be required. The file should be exactly the same.
Hex groups are groups of tiles that are randomized with different amount of hexes and chits. They may also have different settings( such as fog which is to be added later). One scenario in seafarers called "Heading for new shores" has two hex groups. One for the main land, the other for islands around three of the sides. The islands have gold and sea, while the mainland does not. without hex groups, this map would not be possible, since all hexes would be shuffled together and the gold would land on the mainland.
I think it would be easier to edit the amount of hexes and chits on a hex group tab. Your way will allow a non randomized default game.
Thank you for the manual svn up.
Another thought: why do you use pointers for the hex groups, instead of integers in the Hex structure? You could have avoided some tricky pointer arithmetic.
I still have work to do on this, but I am done for the week.
What was I thinking removing the chit number sequence, true I disagree with how it is done, but changing it completely removes any compatibility.
The chit number sequence is restored. Though I only tested it on the map editor. More testing is required. The server probably needs to rebuild the chit numbering sequence before sending params to the client, similar to how the editor rebuilds it prior to saving the .game file.
Tool bar buttons are added, but I stole the image from the terrain buttons. The correct image needs to be used(a colored hex). This is for the hex group tab as well.
Color has been removed from the hex group. Now hex group colors can't be changed.
Placing ports adjacent to a hex group hex is broken. I think I might add a land only option. This will force that there is no sea in the hex group. if the land only option is selected, ports may be facing hex group. This option will effect no setup nodes after fog is implemented.
All hex group options have been removed as that is not the purpose of the patch, though I plan to add them in at a later time. The options in the tab is still there, but empty.
Context menus still need to be added.
Spelling has not even been looked at...
Here are some thoughts:
The map storage could almost be left as-is: Add a * and a group number after the chit.
Example:
h12: Regular shuffled tile (the default)
h12+: Regular non-shuffled title
h12*1: Member of hex group 1
So there is only a need for a minimal change in the stored maps. Furthermore the difference between the maps that are sent to the client (need no change) and the maps on disk for the server will be minimal.
Drawing: see the attached patch to see an example of using overlays to show the hex groups. Each group can draw its own colour on top of the regular tile. The toolbar buttons in the editor can assign tiles to the various groups. ('Assign to group N').
Tile placement: Because the current map structure and the counter per group are equivalent, they can be converted into each other. (They have a mathematical one-on-one relation).
In the editor e.g. a hill hex and an ore hex can be assigned to group 2, which can be shown on the group-tab as Hill:1 Ore;1. When on the group-tab the numbers are changed to Hill:2 Ore:0, the map could be repopulated automatically and be shown in the map-tab. Therefore the map will always contain tiles (instead of the void-tiles you currently have) and the port placement will be easier.
Chit assignment. I think there is no need for a protocol change here. The chits array can be mapped 1:1 to the distribution you've created on the group-tab.
In short: I think the model (the current chits and map keywords) can stay nearly the same, except for the *N addition. The editor will have two views on the same data. (Yeah! Model-View-Controller)
I agree with the change, with the following changes:
h12+: Regular non-shuffled title should be the default when adding a hex with the editor. The hex groups will be the primary way to shuffle the terrain. Therefore there default non hex group should be static. To maintain compatibility the + can remain.
The counter per group can be build from the map. There is no need to send a hex group block except for options. Instead of using a block, I think each option should have it's group prefix, because it matches the rest of the entries in the .game file. (hex-group 0 fog)
Since there will be a default map, the random-terrain option should apply to the hex groups as well.
Port placement will still need to be fixed, if the group has any sea, it is possible that the sea could be randomized at the location the port is facing. A port should not face any hex that is part of a hex group that contains sea. Since the current port placement works, this can be changed in a different patch.
I would propose to keep the current behaviour as much as possible: the editor will per default shuffle the terrain tiles, keep the sea tiles where they are and shuffle the ports separately. -> All terrain tiles are part of 'tile group 1', the sea tiles are implicitly marked non-shuffle and the port tiles are part of 'port group 1'. (For the ports only the resource will be shuffled, not the orientation).
The parameter 'random-terrain' still means shuffle title group 1 and port group 1, but I agree, will means that all the other groups are shuffled too.
For games that currently don't have the random-terrain set, this will means that the game master can force a shuffle for all tiles (or disable shuffling just for tile group 1 and port group 1).
Tile group 1 and port group 1 will not need an overlay in the editor (because they behave as they always did).
The non-shuffle title e.g. could be marked with white or an icon looking like a pushpin (http://www.unicode.org/charts/PDF/U1F300.pdf U+1F4CC). Currently the editor only shows it in the context menu. This display can be in a separate patch (independent of these changes).
The storage can stay the same for the group 1 and non-shuffle tiles.
For groups other than 1, I would suggest the *N notation, as I mentioned earlier (but with N-1, so that a reader of the file will see a 1 for the first additional tile group)
An additional keyword 'shuffle-groups' of type PARAM_INTLIST could be added to specify the additional groups. Any group not mentioned is not shuffled. (Except when random-terrain is true). The numbering can be similar to the *N notation for the tiles.
When fog is implemented an additional keywork 'fog-groups' of type PARAM_INTLIST could be added to specify the additional groups. The default groups cannot have fog. Any group not mentioned will not have fog. I think fog is out of scope at the moment. Feature request 78 (https://sourceforge.net/p/pio/feature-requests/78/) will handle that. Care should be taken to avoid sending secrets to the client. A fogged tile should only be disclosed when the rules state so.
About the problem with ports:
If a board designer places a port pointing to a hex group that also has sea tiles, I think the randomizer should first distribute land tiles to the relevant places before continuing to shuffle the remaining tiles. This could mean in theory up to three land tiles per port, but only the tile with both connection must have a land tile.
In short:
Protocol changes:
These protocol changes can leave all existing game files as they are.
One more thought:
Even in the hex groups I think it would be nice if some tiles can be marked non-shuffle, so that would mean: h12+*2 = Hill, non-shuffle, part of hex group 2.
(A non-shuffle tile will never be shuffled, even when random-terrain is turned on)
The main purpose of the hex groups is to randomize the hexes per groups. With this in mind what would the difference between a tile that is not assigned to a group and and a tile that is assigned to a group but set to not shuffled?
My proposal:
This would simplify the editor and not require the special pushpin symbol to indicate non-shuffle.
Last edit: Roland Clobus 2013-05-30
Hi, sorry for the delay...
Indeed, there is no need to assign non-shuffled tiles to a specific hex group.
I agree with the three proposals with the following remarks:
If we are going to have groups as an overlay, there should be a way to use the default map layout. Instead of 'shuffle-groups', should we use 'non-shuffle-groups'? This implies that the groups are normally shuffled, but allows for them to be used without shuffling.
I think I don't understand your last remark.
This is how I see the current specs/proposals:
Last edit: Roland Clobus 2013-06-04
I was thinking it was about time to rewrite all items. This looks good.
Item 7 answered my last question.
Item 5/6 'random-terrain' should effect all hex groups, not just group 1, but I will go with the way it is currently written. We will find out more after the patch is completed.
Item 16 should be specify that the colors are overlays.
Due to the amount of changes in this patch, the hex group tab will not be included. It is not required to modify the contents of each group. It will still be added later in a different patch.
It was interesting seeing our two separate starting points and watch our opinions change as we approached each other. Looks like we passed each-other, which means that it was a very good and productive discussion.