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

Modernize clothing stores #35940

Merged
merged 25 commits into from
Dec 13, 2019
Merged

Conversation

ymber
Copy link
Member

@ymber ymber commented Dec 7, 2019

Summary

SUMMARY: Content "Overhaul clothing stores"

Purpose of change

Clothing stores feel like they're from 0.A. and they don't use modern mapgen.

Describe the solution

Update clothing stores to use palettes and SUS itemgroups. Add new multi z level clothing store maps.

Depends on #35932.

Testing

Game loads without errors and s_clothes is generated as expected.

@ymber ymber force-pushed the clothing_groups branch 2 times, most recently from dfe989e to 3a59fac Compare December 7, 2019 18:45
@ghost
Copy link

ghost commented Dec 8, 2019

May I ask if you can give us a demonstration of what they will look like now? Or what you have planned for the extra story/floor?

@ZhilkinSerg ZhilkinSerg added [JSON] Changes (can be) made in JSON Map / Mapgen Overmap, Mapgen, Map extras, Map display labels Dec 8, 2019
@ymber ymber marked this pull request as ready for review December 8, 2019 18:32
@ymber
Copy link
Member Author

ymber commented Dec 8, 2019

This should be ready now. I've reduced the scope because it was getting big and full of tangential changes. As it stands every s_clothes* map is updated except s_clothes_3 and s_clothes_5. No new maps have been added and the updates to existing ones are mostly confined to items.
2019-12-08_1913_55
2019-12-08_1914_31
2019-12-08_1915_17

Copy link
Member

@I-am-Erk I-am-Erk left a comment

Choose a reason for hiding this comment

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

Just a few minor things. Excited to see this merge soon.

@ymber ymber requested a review from I-am-Erk December 10, 2019 14:53
Copy link
Member

@I-am-Erk I-am-Erk left a comment

Choose a reason for hiding this comment

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

Some small changes for finesse but overall it could be merged as is.

{ "item": "mag_tailor", "count": [ 1, 10 ], "prob": 80 },
{ "item": "manual_tailor", "count": [ 1, 4 ], "prob": 80 },
{ "item": "tailor_portfolio", "prob": 20 },
{ "item": "tailor_japanese", "prob": 10 },
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should make it one or the other? Not required but it would help a bit with balance

Suggested change
{ "item": "tailor_japanese", "prob": 10 },
{
"distribution": [
{ "item": "tailor_portfolio", "prob": 70 },
{ "item": "tailor_japanese", "prob": 30 }
], "prob": 25
},

This gives roughly the same chance of either appearing, but doesn't allow both to appear.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's reasonable to allow both to appear. A well stocked tailors library is fairly likely to have stuff from around the world.

@ZhilkinSerg ZhilkinSerg merged commit b4bbdbc into CleverRaven:master Dec 13, 2019
@ymber ymber deleted the clothing_groups branch December 13, 2019 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[JSON] Changes (can be) made in JSON Map / Mapgen Overmap, Mapgen, Map extras, Map display
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants