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

Editor: Add some missing terrain groupings #6647

Merged
merged 2 commits into from May 1, 2022

Conversation

Wedge009
Copy link
Member

Resolves #6643.

I'm not overly familiar with the myriad of terrains but this seems to make sense to me (I tested the changes too). I tried to review everything but it's possible there could be more groupings that could be added.

@Wedge009 Wedge009 added Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. Editor Map/scenario editor issues. labels Apr 28, 2022
@github-actions github-actions bot added the Terrain Issues that involve terrain definitions or their implementation in the engine. label Apr 28, 2022
Copy link
Member

@doofus-01 doofus-01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right, thanks.

Copy link
Member

@knyghtmare knyghtmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cases missing:

  1. Elven Winter Castle (found in castle, and not in frozen)

Optionals:

  1. The case of "Craters" being detachable now:
    image

  2. Cave/Dungeon Tileset missing sconces, gates, etc.

  3. And this:
    image

Maybe point number 3 can be dismissed as it's more of a personal preference (since I make dungeon maps mostly)

Context on Case 2:
Originally it was Dd^Dr, but the crater can be detached now and it takes 3 different forms now.

@CelticMinstrel
Copy link
Member

3. And this:

??????

@knyghtmare
Copy link
Member

  1. And this:

??????

point 3 Seems like another issue already reported.

@CelticMinstrel
Copy link
Member

I have no idea what you're even trying to say with point 3. A picture isn't always better than words.

@knyghtmare
Copy link
Member

I have no idea what you're even trying to say with point 3. A picture isn't always better than words.

As in might as well tackle the deprecated terrain collection issue as well.

However, just adding point 1 is enough.

@Wedge009
Copy link
Member Author

Some cases missing:

1. Elven Winter Castle (found in castle, and not in frozen)

Um, that was the first thing I fixed since that was what you reported originally in #6643. Or is that something different? I couldn't find that one, only 'Winter Elven Castle'.

Optionals:

1. The case of "Craters" being detachable now:

I don't know what that means. I could only find one tile as crater - it's currently grouped under desert. I couldn't replicate your image in 1.16.2 release or current master (eb9f463).

2. Cave/Dungeon Tileset missing sconces, gates, etc.

These are grouped under embellishments - I'm not so sure it makes sense to add all the gates to the cave group as well (there are a lot of them) but I added the sconce light to it.

3. And this:

This is from the Secret of the Ancients group? I have no idea about that one.

@CelticMinstrel
Copy link
Member

As in might as well tackle the deprecated terrain collection issue as well.

Eh? Those D's are all deprecated terrains? Deprecated terrains shouldn't be shown at all.

I don't know what that means. I could only find one tile as crater - it's currently grouped under desert. I couldn't replicate your image in 1.16.2 release or current master (eb9f463).

I think what he's suggesting is that crater is now an overlay and should be moved (or maybe duplicated) to embellishments.

@Wedge009
Copy link
Member Author

Wedge009 commented May 1, 2022

I'm really not sure what to do about that - the crater stuff. Unless I have a better understanding, I'll merge what I have here soon.

@knyghtmare
Copy link
Member

I'm really not sure what to do about that - the crater stuff. Unless I have a better understanding, I'll merge what I have here soon.

yeah, whatever changes have been done will be welcomed by map-crafters.

As for the craters...I am a bit unsure about them as I noticed they cause some balancing issues. So, might be better opening another topic.

@Wedge009 Wedge009 merged commit 78ea9ed into wesnoth:master May 1, 2022
@Wedge009
Copy link
Member Author

Wedge009 commented May 1, 2022

Okay, merged, and will back-port to 1.16.

@Wedge009 Wedge009 deleted the editor_terrain_groups branch May 1, 2022 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. Editor Map/scenario editor issues. Terrain Issues that involve terrain definitions or their implementation in the engine.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Terrain] [Editor] Properly assign Terrain types to appropriate terrain groups
4 participants