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

[RDY] Prison mapgen JSONify #20306

Closed
wants to merge 3 commits into from

Conversation

6 participants
@ZhilkinSerg
Copy link
Contributor

commented Feb 17, 2017

Conversion of prison mapgen from map::draw_map to JSON.

@ZhilkinSerg ZhilkinSerg changed the title [RDY] Prison mapgen JSONify Prison mapgen JSONify Feb 17, 2017

"subtype": "collection",
"entries" : [
{ "item": "blanket", "prob": 25 },
{ "item": "jumpsuit", "prob": 35 }

This comment has been minimized.

Copy link
@Reclusive-reptile

Reclusive-reptile Feb 17, 2017

Contributor

Should jumpsuit be replaced with striped_shirt and striped_pants? Prison wardens generally don't like inmates concealing stuff in ample pocket space.

This comment has been minimized.

Copy link
@ZhilkinSerg

ZhilkinSerg Feb 17, 2017

Author Contributor

Jumpsuits were in original mapgen - see https://github.com/CleverRaven/Cataclysm-DDA/blob/master/src/mapgen.cpp#L7354.

I believe we better change loot once all mapgen is moved to json from cpp.

This comment has been minimized.

Copy link
@kevingranade

kevingranade Feb 18, 2017

Member

I agree, open a PR or issue to track updating the prision definition to use new items if you want (I don't think those existed when the prison layout was added).

@ZhilkinSerg ZhilkinSerg changed the title Prison mapgen JSONify [RDY] Prison mapgen JSONify Feb 17, 2017

@Coolthulhu Coolthulhu self-assigned this Feb 23, 2017

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2017

"Visions of solitude" spawned in wrong place - above the ground
The entire lower level has benches on solid rock (visible with a clairvoyance artifact).

Other than that looks good.

@Coolthulhu Coolthulhu removed their assignment Feb 23, 2017

@codemime

This comment has been minimized.

Copy link
Member

commented Feb 26, 2017

This has multiple merge conflicts (with your other PRs that have been already merged).

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2017

I believe it will be better to jsonize mapgen one at a time. I will create new pull request. Thanks, @codemime.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

It's easy to mass-test them, the only problem is that they conflict with each other.

You could have dependent PRs: create one branch with one jsonized mapgen, then create new one on this branch rather than on master. This ensures no conflicts between those two branches.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2017

Thanks, @Coolthulhu. I thought about this, but had some doubts. Branches should be created and merged in certain order, right? Also won't there be conflicts, if I add some changes in parent branch, but won't add these changes to child branches.

@Zireael07

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2017

@ZhilkinSerg You can easily merge parent branch into child branch, though.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2017

Superseded by #20408

@ZhilkinSerg ZhilkinSerg referenced this pull request Jun 9, 2017

Merged

[RDY] Prison mapgen JSONify third try #21211

1 of 1 task complete

@ZhilkinSerg ZhilkinSerg deleted the ZhilkinSerg:mapgen-prison branch Sep 11, 2017

@ZhilkinSerg ZhilkinSerg referenced this pull request May 27, 2019

Merged

Prisoner zombies #30877

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.