Shared kingdom: game internals need some refactoring badly

[SirVer]

I feel most bugs in SharedKingom come through the misuse of the Player paradigm. I am also unhappy in the awkward way teams are currently handled. Both features feel "added in" (which they are of course) instead of "planned out".

In my opinion, Player (that is wl.Game().players) should only be 2 in a 2v2 shared kingdom and 4 in a 2v2 non shared kingdom.
Another paradigm like Controller (4 in shared kingdom and 2v2) is wiser; it would also get rid of the clunky real_player lua function which is kind of confusing. While we're at it, a Game().teams lua property would kill some of the complexity in handling team games atm.

So summing it up:

2v2 (shared kingdom): 4 Controllers, 2 Players, 2 Teams
2v2 (non shared kingdom): 4 Controllers, 4 Players, 2 Teams
1v1: 2 Controllers, 2 Players, 2 Teams.

In fact, the controllers could completely be hidden from the user and the Lua Engine.

Widelands does not understand all these concepts ATM, which clearly reflects in the statistics. I suggest added teams to statistics (via a toggleswitch or so) and updating the game internals to copy with Controllers or Players differently.

Bugs that would vanish by such a design are e.g. 646660, 646657.

[SirVer (sirver)]

nasenbaer, I assigned you because you worked on the shared kingdom feature. I'd really like to hear your opinion.

[Nasenbaer (nasenbaer)]

Your ideas sound like a good solution. However I do not think we have to change the way teams are handled - just to add extra statistics, showing the teams.

Further I would prefare to split this bugreport, as I do not like to invest time for adding team statistics atm. ;)

For the handling of players I am still a bit unsure about the best solution.
If we do it the way you suggest, it would be harder to get the starting position of the "shared in" players and not just only some parts of the network engine, but as well the savegame handling would need a bigger code change.

So my suggestion would be to add a "initialized" flag to players, which could be set via the lua initialization (or generally via lua in scenarios).
If a player is not initialized it would not show up in statistics (which would as well be a nice addition for scenarios, where complete economies are set at the moment the player reaches a specific area and only after that the new player would show up in statistics).

Of course this sounds like a wrapper for a feature, but in fact, that's like I wanted it to be, as shared kingdom mode is not the common way to play Widelands and i liked the idea to be able to see which players are combined (you can read from the savegame files, how which player was initialized and whether the player was in shared kingdom mode (== had a partner). During the implementation i just thought about "what does happen" and my answer was "it's like player B surenders to player A and player A therefor gives him the right to reign the land" - and that's exactly like it is handled at the moment :)

Of course this is a game design question and I am sorry, that I haven't explained my thoughts behind the implementation before!

So what do you think?

[SirVer (sirver)]

I think we should add teams to avoid the repetitive gathering into teams in the win conditions Lua scripts. I am not so strong oppinioned about this. I agree that adding statistics for teams is worth another bug report.

About your third paragraph: If there are only 2 players in a shared kingdom 2v2, then savegames would not change at all: just save the game as it would be a 1v1. You could then also load a 1v1 game and continue it as kingdom together and vice versa. Only the network and controlling mode would need to understand that even though there are only 2 players, there could very well be 4 or more *controllers* and even more observers.

4th paragraph: I think such an flag might be useful for scenarios... something like: show_in_statistics. But I feel it is a hackish solution to just hide the player that have no buildings in the statistics. They are not actually players in the game sense and should not be treated as such.

5th par - end: No problem not discussing the implementation; actually it is working fine at the moment so this is good :). But I think the design idea of carrying around 2 dead players that have nothing to control is just not correct. If we play monopoly with 4 guys, but 2 each share money and cards it is technically a 2 player match. One of the players could leave and the game would be absolute exactly the same. In fact they could resume the very next day the very same game playing 4v4. Shared kingdom is not a start condition, it is a mode of control. A really good implementation would make this distinction and would therefore also allow to resume savegames as shared kingdom. No change in the savegames should be needed for this, but some stuff how a network game is started and configured would need changing I'm sure.

[Nasenbaer (nasenbaer)]

well at the moment I see 2,5 possibilities (beside a major major major rewrite ;) ):

1) We do not initialize the players (like as if the player position was set to "closed"), but save a "partner" value (or similiar) at their position, so the game knows, that the position is for shared kingdom instead of a real player - technical this could be done similiar to the game preload data set - we initialize the basic player data to get it's starting position and remove it afterwards (but i am not sure if saving works than - if not we are back to "keep it hidden and unused and only use it for savegames" or "rewrite some parts of the savegame code")

2) complete rewrite of the launchgame menu -> the host can add slots, players can take a slot and set their values (player number x (with tribe, team, etc.) or spectator) and the host can assign computer players to a slot as well. Players/Starting positions that are not assigned will not be initialized. (that will as well change the way "shared kingdom" is working, as only 1 starting position will be present for all clients that control the player - actually not what I would like to)

3) the same as 2) just that we convert map "players" to map "starting positions" - would behave the same as the current implementation, but would make the UI even more complex + some more code would have to be changed (e.g. "home" button takes the player to it's starting position - but what starting positon ;) ) ?

not included the work in network code, but I am relatively sure, that it's nearly the same amount of work for all 2,5 possibilities.

So... to be honest, I am still not conviced that we should change the behaviour. Of course I understand your point with "it looks like a hack" - however why shouldn't we do it that way, if it's the simplest way to do so and it works fine as it is without any downsides?

It's like with luminescent tubes: it's to expensive and complex to use a gas that directly emits visible light, so we use powder on the tube to let it glow through the "invisible" light. ;)

Okay slightly different, but as I wrote, I am still not convinced, that it is worth to change the code. :)

[SirVer (sirver)]

I like solution 2 the most, I agree that the UI needs to be thought through very carefully so that it is natural to use. Maybe one could split the startgame menu into two steps: shared kingdom, and number of starting positions -> teams and slots.

I am mostly concerned about the Lua code frankly; there something must happen: the sorting of teams and the constant special casing of them is bad enough, but the real_player function is so counter intuive and hackish. I first and foremost want to keep this area of widelands clean as it is in a very good shape at the moment.

Lua should only see two players. And as it is currently, a player has no start position anymore, but wl.map.Map() has a property called player_slot which contains the start condition. One could change the start condition code so that the lua function gets a player and a player slot and will use those for intialization; one could then retain the shared kingdom behaviour as is.

The reason to change something is that stuff keeps easy to add in. Widelands has always just been added to and the code is a terrible mess for that very reason. To change something fundamental in widelands is a painful process atm (for example removing Trigger & Events and adding lua, or for that matter make Player Infrastructure work in the Editor). We cannot change the past, but new stuff must be added in carefully to not let the code rot further, even if it means more work (which it doesn't in the end; it just means that the one adding features in is not the one shouldering the burden of maintenance).

[Nasenbaer (nasenbaer)]

Okay let's stop discussing. :)

So my way to fix this would be:
* split launchgame menu and playerdescgroup
  -> LaunchSPG should basically stay the same, just that all the multiplayer clutter, chat and share kingdom stuff is removed, same applies to playerdescgroup where ready checkboxes and share kingdom functions get removed.
  -> LaunchMPG will get a new ui widget for clients -> every client gets assigend to a slot fitting to the nethost client number (which should make handling much easier) and is able to set a player or spectator mode (as described above) - if a player is already selected elsewhere, the init condition will automatically be set to "help ..."

* the "partner" value I introduced will be removed again.

* As soon as those parts are working fine, we can add some more features to the LaunchMPG like a checkbox to allow / disallow other player to join in "your" player.

[SirVer (sirver)]

sounds good. I set this to triaged then.

[Nasenbaer (nasenbaer)]

Okay, here is my new mockup - a bit different to the one I discussed with Holger, still without tabs and with some space saving through using icons instead of Names (tribes, human/computer/closed button)

The info stuff on the right side is already ready and functional - the button besides the map name is only visible for host players. If the host clicks on it, a popup opens asking whether a map or savegame should be load + a cancel button.

I would like to hear some opinions about this mockup, before I waste my time, so ... :)

Further things to be discussed:
* Ready boxes yes/ no? The are at least needed by the dedicated server (how many people use it? anyone interested in completing the dedicated or should we rip it out and remove the ready boxes? )
* I would like to add a fourth state for the player boxes on the right (besides human, computer and closed): shared in, which means - the player is closed and not used, but it's starting position is used as second or third or ... starting position of the selected player (like the standard in the current shared kingdom version)

[SirVer (sirver)]

Wow! I really like the mockup, something to discuss about! good work so far.

1) the AI button is not very intuitive, but it can obviously be replaced. It is most likely only for the mockup, isn't it? ;)
2) I am all for removing the ready boxes. If something is needed for dedicated servers (which is something that I would not invest a lot of energy right now!), I'd suggest adding a chat command /start to trigger the launch instead of having all players need to click some checkbox!
3) I like the idea with the dialog box for savegame <-> map loading!
4) I am not too happy with the ui suggestion of the shared-in idea (your second '*' point). But I've got no better idea how to design this. What happens to the tribe button when shared in is selected? Is it grayed out and set to the player tribe?
5) The defaults (that is, if noone clicks anything except for the host selecting a map) should be that a free-for-all game is started when start game is pressed.

All in all, I think this UI is significantly better than what we have currently. I think it also handles all cases that we want to model. Very good work, Nasenbaer!

[SirVer (sirver)]

aaah.. bad wording:

1) i meant the graphic for the AI. Not the button.

[Nasenbaer (nasenbaer)]

Answer on #9 and #10:

> 1) the AI button is not very intuitive, but it can obviously be
> replaced. It is most likely only for the mockup, isn't it? ;)

indeed ;). I will try to create some more intuitive graphics for this :)

> 4) I am not too happy with the ui suggestion of the shared-in
> idea (your second '*' point). But I've got no better idea how to
> design this. What happens to the tribe button when shared
> in is selected? Is it grayed out and set to the player tribe?

My idea was, to use that button to select the player that this starting position is added to. So it would show the flag. But to be honest this is not really intuitive. I think we can search for better solutions. That part of the UI will definitely be created after all the other stuff is ready.

One thing I forget is the "?" button (in the top right corner?) - if clicked it should open a popup help explaining the basics of the UI.

[SirVer (sirver)]

Released in build16-rc1