silverua / slay-the-spire-map-in-unity

Implementation of the Slay the Spire Map in Unity3d
MIT License
288 stars 68 forks source link

Changed the path generation algorithm. #19

Closed hojjatjafary closed 1 year ago

hojjatjafary commented 1 year ago

Pull Request as discussed in: https://github.com/silverua/slay-the-spire-map-in-unity/issues/18

silverua commented 1 year ago

@hojjatjafary Thanks for making the PR! The path generation seems to be working well, but one regression that I noticed is - it ignores the code that gets rid of the X paths, so we can now get this: image

If you know an easy fix for this, feel free to edit the PR. I am traveling till next Tuesday, Dec 13th. Can have a look after 13th and check how to restore X paths elimination.

hojjatjafary commented 1 year ago

Seems the cross edge issue is fixed.

silverua commented 1 year ago

The cross paths issue is fixed. Thank you!

Sorry if I seem overly picky. I was testing it a bit more after the change and noticed that:

Any thoughts on this?

hojjatjafary commented 1 year ago

The previous algorithm generated many paths in a while loop, I commented in the code where you can add a parameter to generate more paths so you will see more branching and feel more randomness.

silverua commented 1 year ago

To test this out, I've generated 5x more paths by making a small local change: int numOfPaths = 5 * Mathf.Max(numOfStartingNodes, numOfPreBossNodes);

It helped introduce more branching, but the pattern "long single path in lower lane" is still very common:

image

For example, during this test run, I generated 14 maps. 10 of them had this long lower path. 10/14. Which seems like a lot. I think, it may become a predictable pattern for the users if this is kept as-is.

hojjatjafary commented 1 year ago

Should be fixed now, there was a small issue in random generation.

silverua commented 1 year ago

Thank you! The recent change looks really good. I pushed a commit with the change that you suggested to add an option to generate more paths into the map config. It added a bunch of whitespace changes, so please tick "hide whitespace" when viewing to only see the actual change that was made. Let me know how you find it. I'm happy to merge at this point.

hojjatjafary commented 1 year ago

It is better if the config value just added up to the maximum value, we need to generate at least 'max(startingPoints, prebossPoints)' paths to have a valid map.

silverua commented 1 year ago

Sorry for the delay. Just got back home. How about this version? (I.e. - adding more paths instead of overriding the total amount)

hojjatjafary commented 1 year ago

Looks good.

Chizaruu commented 1 year ago

Close #18 since it's merged :)

silverua commented 1 year ago

@Chizaruu Closed