Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no_leader=yes broken if the map file has a starting position for this side #2604

Closed
2 of 3 tasks
sevu opened this issue Mar 6, 2018 · 10 comments
Closed
2 of 3 tasks
Labels
Bug Issues involving unexpected behavior. Regression Issues that were not present in previous releases.

Comments

@sevu
Copy link
Member

sevu commented Mar 6, 2018

This are three related bugs, one is a regression, one is fixed in 1.13.
Following situation. You have a map designed for 6 players, with six starting positions.
You have two scenario files which use this map

One scenario has a seventh side for an AI unit with no_leader=yes and hidden=yes. This one

  • shows up as 6p map in MP Create
  • shows 6 player slot in MP Staging (the screen after MP Create)
  • In the game the seventh side has no leader. As expected

The other scenario file is for three players, and the AI is side 4. In difference to the other scenarios the sides are 1-3, 4 is the AI. The leaders for side 1-3 are placed with the x,y keys, ignoring the starting position defined in the map file. Side 4 has no_leader=yes and hidden=yes.

  • shows up as 5p in MP Create. That's a bug. Why assumes it 5 players? It could be because side 3 starts at the position which is in the map file the starting position of side 5. But it doesn't seem to be, happens when using a random other place too. Same in 1.12.
  • shows 3 player slots in MP Staging. In 1.12 it wrongly showed 1,2,3 and 5,6. 4 probably not because it is now the hidden AI side. Happily this is already fixed.
  • In the game the AI side has now a leader, starting at the position which is defined in the map for side 4. This does not happen in 1.12

That's the UI part which I mean in MP Create, in case it's unclear.
player_count

@sevu sevu added Bug Issues involving unexpected behavior. Regression Issues that were not present in previous releases. labels Mar 6, 2018
@gfgtdf
Copy link
Contributor

gfgtdf commented Mar 6, 2018

I an not 100% sure what this report is about so i'll just say what i know:

The beviour of no_leader was changed in 1.13 during the spmp patch:

It now just means that the mp initilisation code will not generate a default leader. This means if your side has a type= defined, the presence of type= will take priority over the no_leader attribute. in 1.13 no_leader defaults to true for [side]s in [scenario] but not for [side]s in [multiplayer]. In particular no_leader should no longer be used in sp, just omit the type= attribute.

shows up as 5p in MP Create. That's a bug. Why assumes it 5 players?

It probaly does a calulcation like this: 1) 6 starting positions and one [side] with controller=ai gives 6-1=5 sides.

@sevu
Copy link
Member Author

sevu commented Mar 6, 2018

The Ai side has no type= attribute either... It pretty much has nothing besides some [unit]s
Full code looks like this: https://bpaste.net/show/0c01247fcf2a

I have a look at your change in an hour.

gfgtdf added a commit that referenced this issue Mar 6, 2018
it previously only worked if both leader_lock and no_leader were set to yes.

from looking at the code i wonder what currently is the point of no_leader at all. I think no type= attribute together with leader_lock=yes should have the same effect? 

#2604
@Vultraz
Copy link
Member

Vultraz commented Mar 7, 2018

Looks like that number is the number of sides that a player is allowed to control:

		num_players_ = 0;
		for(const config& scenario : data_.child_range("side")) {
			if(scenario["allow_player"].to_bool(true)) {
				++num_players_;
			}
		}

I'm not sure how that's the wrong behavior.

@gfgtdf
Copy link
Contributor

gfgtdf commented Mar 7, 2018

what he is surpoised about is that the game engine generates sides for each starting position in the map, so if your scenario has 4 [side]s but your map has 6 starting positions, the engine will add 2 [side]s to the scenario wml.

@Vultraz
Copy link
Member

Vultraz commented Mar 7, 2018

Well that kinda makes sense. Why have a starting position without a side.

@gfgtdf
Copy link
Contributor

gfgtdf commented Mar 7, 2018

Well that kinda makes sense. Why have a starting position without a side.

in his case becasue he wanted to reuse a map file for multiple mp scenarios (one beeing for 3 players the other beeing for 6 players)

@gfgtdf
Copy link
Contributor

gfgtdf commented Mar 7, 2018

i don't really have a strong opinion on whether to keep that behviour or not, but in any case it should be properly documented.

@Vultraz
Copy link
Member

Vultraz commented Mar 7, 2018

BTW, I also don't understand the bug he says is fixed.

@gfgtdf gfgtdf closed this as completed in c64c6f5 Mar 7, 2018
@gfgtdf
Copy link
Contributor

gfgtdf commented Mar 7, 2018

this is marked as fixed becasue the main part 'no_leader=yes broken' is fixed, #2605 is still open.

jyrkive pushed a commit to jyrkive/wesnoth that referenced this issue Mar 9, 2018
@jyrkive
Copy link
Member

jyrkive commented Mar 9, 2018

I have now merged #2605 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. Regression Issues that were not present in previous releases.
Projects
None yet
Development

No branches or pull requests

4 participants