Game setup / civ selection improvements

[historic_bruno]

• Adds player data to map files

• Adds support for civ data including "subfactions"

• Adds support for other map types (random, saved?, etc)

• Player options displayed in setup

• Player entities initialized by script (instead of engine)

• Generalizes XML loading for scripts

• Civ info displayed in dialog

See discussion here.

[fcxSanya]

As far as I understand your patch waits for review, so I added the "review" keyword (see SubmittingPatches)

[fcxSanya]

I tried to see to your patch and got following errors:

ERROR: CXeromyces: Parse error: civs/cart.xml:17: 
Input is not proper UTF-8, indicate encoding ! Bytes: 0x92 0x73 0x20 0x63

(same for all other civs) - it seems like these files encoded as ANSI, when I converted it to UTF8, error disappeared.

ERROR: Texture failed to find source file: 
"art/textures/ui/global/icon/info_black.dds"

(same for info_white.dds) - probably you forgot to add this files

Failed to find font 'serif-bold-20'

(same for serif-bold-22) - all other fonts have fnt and png files, but these two - only fnt files, probably you forgot to add pngs.

[Philip Taylor]

Patches can't contain binary files - probably best to just upload them as separate attachments if there's not too many. The UTF-8 thing is likely an issue with the patch system too, but sounds like it's easy enough to fix after applying them. (This is where a DVCS would be a better tool for sharing changes...)

I'll try to look over this patch soonish when I have time (maybe a few days or a week or so). Any comments/fixes before then would be appreciated :-)

[historic_bruno]

Yes, I use two bigger fonts than were available (will attach the pngs now) and two new icons.

As far as the encoding issue, I think the problem is that the civ wiki (source for the civ data) contains some weird characters which are not even UTF-8 and had to be removed. By default the XMLs may not be UTF-8 encoded, a patch probably can't change the encoding?

[fcxSanya]

Are tooltips on hover above info icons on civinfo page not implemented yet, right? All the rest seems working good for me.

[historic_bruno]

Correct - icon tooltips aren't implemented.

[historic_bruno]

I'm modifying the patch a bit, changes so far:

• More complete support for random maps

• Game options displayed in setup (reveal map, victory conditions, etc)

• More complete support for teams and diplomacy

• Chat messages and names are now escaped to prevent special chars

And some bug fixes. Should have an update in the next few days.

[historic_bruno]

New patch, includes all the above and:

• Implementation of icon tooltips: new method MouseOverIcon() of CText (text boxes) is tested by GUItooltip, prior to setting the tooltip properties. When called it iterates through all the icon sprites in the text box, checking their areas with the current mouse position. If icon is found with tooltip under the mouse, _icon_tooltip properties are set on the text box. If set, those tooltips take priority over the normal tooltip, but if not over an icon, the normal tooltip would be displayed (if any).

• Also required slight modification of tag parsing by GUIstring: instead of assuming text tags only have one additional attribute at most (e.g. icon displace), all extra tags are placed in a vector. Vector includes attribute and value pairs. This is much more flexible IMO.

• For diplomacy, preset array (from a scenario) takes precedence over team numbers, but otherwise team numbers are used to build the array. If lacking diplomacy data, then it's set to all enemies. See also ​#7: Diplomacy

[Philip Taylor]

Looks great! A few thoughts from first checking through this patch:

I'm not convinced that single-player saved games should be part of this interface - it seems needlessly complex, compared to clicking a "load" button on the main menu then clicking the game to load. Seems good for multiplayer saved games though, since that will need players to connect and coordinate and select slots etc, so it can share most of the multiplayer setup code. Since we don't have saved games implemented, probably the option should just be removed from the UI now, and we can add it back to multiplayer modes when it's needed.

Victory conditions should default to Conquest.

Running the "test" project gives an error:

ERROR: JavaScript error: simulation/components/GuiInterface.js line 41
TypeError: cmpPlayer.GetTeam is not a function
  ()@simulation/components/GuiInterface.js:41
  ()@simulation/components/tests/test_GuiInterface.js:52

(which should be easy to fix, the test script is just a bit fragile).

civinfo.js:

getXMLList shouldn't bother checking for filenames starting with "_", since we don't expect to have any for civs.

getCivDisplayName should raise an error if civData is undefined or doesn't have a Name, because that should never happen, instead of trying to return the filename. (Looks like the only case is if the map settings specify an unrecognised civ, which is an error - silently accepting it will just make the bug harder to find.)

heading(): "if(textArray[i].length > 3)" seems very English-centric, and we want to (eventually) support other languages equally well, so we shouldn't hard-code things like this. The text should probably just be correctly capitalised in the data files.

civinfo/styles.xml: there doesn't seem much point adding that empty file.

gamesetup.js: The switch indentation shouldn't be changed. (It's intentionally indented like ​this, since that's a common style, and also the changes make the patch harder to read.)

Does escapeText really need to escape \ characters? (I thought [ was the only character triggering special behaviour in the GUI). Also it probably needs to do something sensible with messages containing newlines, so they don't mess up the display.

There's duplicated code between civinfo.js and gamesetup.js, for loading civ data. If possible, it should be moved into a shared file (in gui/common/) and then loaded by both GUI pages.

selection_details.js: "civName == "Gaia"" - names will probably be localised in other languages, so they shouldn't be compared as strings. Need some other way of identifying whether it's Gaia.

Player.js: needs some documentation of what this.team means (e.g. are two players in team -1 considered to be the same team?), and what this.diplomacy means (what are the array elements?)

MapReader.cpp: The commented-out sections should be deleted if they're not used any more. But I don't think CMapSummaryReader should be deleted: all the map-reading code ought to be in MapReader.cpp, because that's the clean/logical place for it to go, not in gui/scripting/ScriptFunctions.cpp (where it's been moved to).

CText.cpp: Is it really necessary to add new _icon_tooltip settings, or would it be sufficient (and simpler) to just replace the normal tooltip values in this icon case? (I don't think we'll ever want a normal tooltip plus icon tooltips on the same object.)

GUItext.cpp: Commented-out section should be deleted.

Code style nit-picking:

for(var n in civFiles)
if(civSettings.Name)
switch(g_GameAttributes.mapType)

Should put a space before "(" in all those kinds of cases, for consistency with the rest of the code.

//Udate civ gameplay display

Should put a space after "" in all those kinds of cases. (Also, typo in that specific case.)

// Called when user selectes number of players

Typo.

civinfo.js: string.substring(1, string.length) - could be just string.substring(1) (which is shorter and also avoids confusion over whether the second argument is index or length).

Setup.js: var player; etc should be declared in the tightest block where they're needed. (JS doesn't do block-scoping but I think it's helpful to act like it does, as documentation of the scope where a variable will be used).

CText.cpp:

SGUIText* guitext = (SGUIText*)*text_it;
SGUIText::SSpriteCall spritecall = (SGUIText::SSpriteCall)*sprite_it;

Don't need the casts there.

[historic_bruno]

Thanks for the suggestions Philip, the patch has been modified a lot, and it should be in line with most of them now.

CText.cpp: Is it really necessary to add new _icon_tooltip settings, or would it be sufficient (and simpler) to just replace the normal tooltip values in this icon case? (I don't think we'll ever want a normal tooltip plus icon tooltips on the same object.)

Can't say I agree there, because if we overwrite the tooltip value, then the behavior is undefined to the user. There may be times when we do want tooltips on text boxes, but it's not known ahead of time if there will be an icon w/ tooltip in the text.

[Philip Taylor]

I'm happy to apply this now the release is out of the way. I can merge it with the current code and new maps etc, but if you've made any further changes then that might cause conflicts - should I commit this patch or do you have an updated patch that should be committed instead?

[historic_bruno]

Replying to Philip:

I'm happy to apply this now the release is out of the way. I can merge it with the current code and new maps etc, but if you've made any further changes then that might cause conflicts - should I commit this patch or do you have an updated patch that should be committed instead?

Philip, I've fixed some bugs and made a few aesthetic changes (basically bringing it up to the latest screenshot in the forum discussion). It's probably wise to hold off for a few days, until I update to the latest rev. and check it all again. Then I'll upload a new patch here and merging should be fairly smooth :)

[historic_bruno]

Latest patch incorporates the following:

• Default player data now in simulation/data as player_defaults.json - used during setup when we don't already have player data (incomplete scenarios, or random maps)
• Player color boxes in game setup
• Multiplayer chat shows usernames in player colors
• Player assignment boxes are better centered
• Better handling of older maps (which should never be used now anyway)
• Setup teams and diplomacy
• Show some basic settings in game setup (victory conditions, reveal map, etc)

[historic_bruno]

• Fixed bug where certain components were being init'd before they should have been. Setup scripts have been separated into "LoadPlayerSettings" which creates player entities only and is called before terrain/entity parsing, and "LoadMapSettings" which does all the other component init based on map settings (victory conditions, stance, circular map, etc) and is called only after terrain/entity parsing.

[ben]

(In [8494]) Game setup changes, including showing name, civ, team, and some options loaded from the scenarios. Civ data added. Civ info window to preview civs. Icon tooltips. Support for random map implementation. Fixes #590.