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

Improvements to the Lua map generator #993

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

CelticMinstrel
Copy link
Member

No description provided.

@CelticMinstrel
Copy link
Member Author

Anyone have any idea why the latest commit doesn't work? There's two problems with it:

  1. If I use map:set_tile like the comment says it should, I get an assertion error saying that the map's width is broken (there are more or fewer terrains in some row than the map width).
  2. With the current implementation, the road is patchy and disconnected.

@Vultraz Vultraz added this to the 1.13.8 milestone Apr 27, 2017
@CelticMinstrel CelticMinstrel modified the milestones: 1.13.9, 1.13.8 May 12, 2017
@CelticMinstrel CelticMinstrel removed this from the 1.13.9 milestone Jun 13, 2017
@CelticMinstrel CelticMinstrel added this to the 1.14.1 milestone Jun 25, 2017
@gfgtdf
Copy link
Contributor

gfgtdf commented Aug 7, 2017

@CelticMinstrel you have changed the milestone to 1.14.1 but since it has changes that add new syntaxes/features this cannot be done during a stable release. We could eigher make it before 1.14 or at 1.15 .

EDIT: since we already have some conability breaking changes in 1.13 with cave map generators i'd really prefer doiing this in 1.13 . If it's just the last commit that doesn't work we coudl alos merge the other commits already and postpone that one for 1.15

@CelticMinstrel
Copy link
Member Author

CelticMinstrel commented Aug 7, 2017

@gfgtdf – Ultimately I wanted to have a multiplayer cave map which basically shows off the new cave generator features, and I believe the road cost thing is basically needed for that. I think the patchy and disconnected aspect of the road is probably something about order of operations (as if the road is being added correctly, but then something else is overwriting it later).

EDIT: Oh, the problem with using set_tile might be that it doesn't verify that the location is on the map.

@CelticMinstrel
Copy link
Member Author

Okay... there are still some things I want to do (mainly adding user config option on the cave map), but I think this could be mergeable at this point.

The road algorithm does seem like it could use some improvement, mind you - the road I'm getting is pretty patchy. Tweaks to the scenario WML for the cave map would also be welcome (maybe @sigurdfdragon would have some ideas on improving it?).

@sigurdfdragon
Copy link
Contributor

I may be able to take a look at this on Thursday.

@sigurdfdragon
Copy link
Contributor

sigurdfdragon commented Aug 18, 2017

@CelticMinstrel The things that may fix the patchiness are:

  1. There isn't a [road_cost] entry for all types of terrain that might be encounter. At least Sm (muddy swamp) and Rb^Fetd are listed as clear terrains in the generator, and there's no road cost them. Try adding those and looking for others. The gist is to make sure there is a [road_cost] tag for anything the generator may encounter from point A to B.

Also Ur^Ii & Ur need to have an entry at the bottom of the list of converting into themselves with a cost of 2

  1. Try more standard road costs for various terrains (ie, 10, 20, 25, 30 etc) Use 50 as the highest. Use 2 as the lowest amount. I don't know why / don't remember why mainline doesn't use 1 for MIN_ROAD_COST macros, but there's probably a reason.

@CelticMinstrel
Copy link
Member Author

Thanks for the tips. I'll try applying these suggestions, probably tomorrow.

@CelticMinstrel
Copy link
Member Author

Those tips seem to have worked, thanks! I think this can now be merged; I do still want to extend it to allow up to 9 players (or at least 8; 9 might be a bit hard), and to allow some configuration of parameters, but aside from that it all seems to be fully-functional, and the map it generates doesn't seem to be too terribly boring either. At most it could use maybe a few tweaks, maybe making the lake bigger, adding a river, or something like that.

@CelticMinstrel
Copy link
Member Author

...okay, I spoke a little too soon. There's this weird bug whereby the map (sometimes?) doesn't generate until you click "Regenerate". That is, at first you see nothing and it says the map is 0x0. Click "Regenerate", and it all works.

@gfgtdf
Copy link
Contributor

gfgtdf commented Aug 27, 2017

please make the return value of the lua user_config optional.

@CelticMinstrel CelticMinstrel changed the title [WIP] Improvements to the Lua map generator Improvements to the Lua map generator Aug 27, 2017
@CelticMinstrel
Copy link
Member Author

Okay, so basically all working now, I guess. There's still some room for improvement, mind you; if you make the lake large enough, you'll get villages dotted through it, for example. I also got twice as many castles as players, for some reason.

@CelticMinstrel
Copy link
Member Author

In other words, unless @sigurdfdragon or others want to suggest some tweaks, this can be merged whenever.

@sigurdfdragon
Copy link
Contributor

I'll be able to take another look at this on Wednesday.

@gfgtdf
Copy link
Contributor

gfgtdf commented Aug 30, 2017

just to be sure, this doesn't change behaviour of the 2 mainline scneraios where the cave random map generator is used, right?

@CelticMinstrel
Copy link
Member Author

It shouldn't, since it's only adding additional capabilities to the generator and using them in a brand-new MP scenario. Still, it wouldn't hurt to check, just to be sure...

(That said, I'd argue that we should consider using some of these new capabilities in the two mainline scenarios. With these upgrades it could be possible to get rid of all those SCATTER_EMBELLISHMENTS macro calls in the HTTT scenario, for example. However, that's outside of the scope of this PR.)

@sigurdfdragon
Copy link
Contributor

I found the following bugs:

  1. The number of villages produced on HttT 17 & SoF 4 is consistently lower than on current master. (ie, around 6-7 on SoF 4, whereas on master approx 15-35) The villages that are produced are at the bottom of the map, and the count thins out when you go up. This is most obvious on HttT 17
  2. The double keeps seem to only start occurring when the settings box is opened, regardless of wether or not anything is changed. Once it happens, it's sticks, until the create game screen has been exited and re-enterted.
  3. <illegal tile in map (2 Ko) '2 Ko'> (or similar) occurs occasionally on regenerating. The smaller the map & higher number of player seems to make it more likely to occur? My guess is that it might be trying to put a keep on top of an existing keep? (fixing double keep probably fixes this)
  4. 'Roads between castles' checkbox does not appear to work, as there are still roads when the checkbox is removed.
  5. Not sure if it is intentional or not, but the roads generated look to be a width of 3, whereas roads on the default map generator are typically a width of 1 hex.

Lake villages - A way of dealing with them could be to add a prestart event to convert villages surrounded by water into merfolk villages.

@CelticMinstrel
Copy link
Member Author

The roads were intentionally wide, with width=2... not sure why they're 3 wide though when I'm asking for 2 wide. I'll look into these issues, maybe this weekend or early next week.

@Vultraz
Copy link
Member

Vultraz commented Sep 6, 2017

Do you still have issues with the settings dialog?

@sigurdfdragon
Copy link
Contributor

I should be able to check the settings issue post 86f0262

by the end of the week

(This is the same API that World Conquest uses for its map generation.)
Every place it's called already passes a default-constructed config, so this won't affect any existing code.

The removed assert is pointless when it'll be cleared by the call, and the two places that called that function also passed a default-constructed config already, so it was doing nothing.
- Detect obvious cases of inheritance loops
- Don't try to keep track of tags with a super tag that exist within a conditional tag
- When searching for a tag path, don't append the leaf to the full path
@CelticMinstrel
Copy link
Member Author

Updated todo list (copied from #993 (comment) and deleted completed tasks):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign Lua API Issues with the Lua engine and API. MP Issues with multiplayer support or bundled multiplayer content. Schema UI User interface issues, including both back-end and front-end issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants