Skip to content

/Cataclysm-DDA

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

Emergency bugfix for experimental 4938 #16906

Merged
merged 4 commits into from May 28, 2016

Conversation

Reviewers
No reviews
Assignees

@Coolthulhu Coolthulhu

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@BorkBorkGoesTheCode @Cyrano7 @Podesta @Coolthulhu @kevingranade
@BorkBorkGoesTheCode
Copy link
Contributor

commented May 28, 2016 •
edited

Fixed the horrible mess I made.

@BorkBorkGoesTheCode BorkBorkGoesTheCode referenced this pull request May 28, 2016

Closed

experimental 4938 bugs and a fix #16907

@BorkBorkGoesTheCode BorkBorkGoesTheCode changed the title Emergency bugfix for m249 Emergency bugfix for m249 and m60 May 28, 2016

@BorkBorkGoesTheCode BorkBorkGoesTheCode changed the title Emergency bugfix for m249 and m60 Emergency bugfix for experimental 4938 May 28, 2016

@BorkBorkGoesTheCode BorkBorkGoesTheCode referenced this pull request May 28, 2016

Merged

[RDY] Update guns and ammo #16863

@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

Would @mugling @Rivet-the-Zombie or somebody please merge this?
@BorkBorkGoesTheCode
Bugfixes 
Bugfixes

Reordered the rail rifles

Removed burst from m60

Removed buggy burst mode

Fixed typo

Added copy-from

Fixed my copy-from bug

Fixed magazine

I am deeply sorry.
Loading status checks…
5df7217
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

Are you still online @Cyrano7 ?
@Cyrano7

This comment has been minimized.

Sign in to view
Copy link
Member

commented May 28, 2016

I don't have merge rights.
@Cyrano7

Cyrano7 reviewed May 28, 2016
View changes

data/json/items/gun/9mm.json Outdated
@@ -29,6 +29,13 @@
[ "BURST", "3 rd.", 3 ],
[ "AUTO", "auto", 5 ]
],
"magazines" : [

This comment has been minimized.

Sign in to view
Copy link
@Cyrano7

Cyrano7 May 28, 2016

Member

Although you don't need this space between " and :
@Podesta

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 28, 2016

Make a post over at reddit/forums telling people not to update to it. It'll probably get merged in a couple hours.
data/json/items/gun/9mm.json
@@ -29,6 +29,13 @@
[ "BURST", "3 rd.", 3 ],
[ "AUTO", "auto", 5 ]
],
"magazines": [

This comment has been minimized.

Sign in to view
Copy link
@kevingranade

kevingranade May 28, 2016

Member

How is this a bugfix?

This comment has been minimized.

Sign in to view
Copy link
@BorkBorkGoesTheCode

BorkBorkGoesTheCode May 28, 2016

Author Contributor

My PR removed the magazines by accident

This comment has been minimized.

Sign in to view
Copy link
@BorkBorkGoesTheCode

BorkBorkGoesTheCode May 28, 2016

Author Contributor

Starting a world in experimental 4938 produces errors related to ammo/222.json, ammo/545x39.json, ammo/9x18.json, ammo/metal_rail.json, gun/223.json, gun/308.json, and gun/9mm.json.
data/json/items/gun/metal_rail.json
@@ -1,32 +1,5 @@
[
{
"id": "rebar_rifle",

This comment has been minimized.

Sign in to view
Copy link
@kevingranade

kevingranade May 28, 2016

Member

why?

This comment has been minimized.

Sign in to view
Copy link
@BorkBorkGoesTheCode

BorkBorkGoesTheCode May 28, 2016 •
edited

Author Contributor

For some reason git rebase put that there. The reordering is part of the original PR
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

@Coolthulhu are you online now?
@Coolthulhu

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 28, 2016

Yes, I'm trying to find out what's the deal with all the broken JSONs.

@Coolthulhu Coolthulhu self-assigned this May 28, 2016

@Coolthulhu

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 28, 2016

Still warns about missing magazine for one gun
@Coolthulhu

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 28, 2016

The bugged item is 545_ap - it is missing a symbol and has wrong ammo type.

@Coolthulhu Coolthulhu merged commit c854d4e into CleverRaven:master May 28, 2016
1 check passed

1 check passed

default
Details
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

That is really weird. The copy from process worked before
@Coolthulhu

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 28, 2016

I added a fix on my side. For now just copied the relevant values manually.
Master branch should work now.
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

Closes #16907
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

Thank you.
@kevingranade

This comment has been minimized.

Sign in to view
Copy link
Member

commented May 28, 2016

I'd like answers to my questions about parts of this change. Several pieces looked like non-bugfixes, if you're making an "emergency bugfix" you should never include additional changes.
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

For some reason git rebase decided to do strange things, probably because I used the wrong setting. I do not have enough experience with git to tell you what it was doing.
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

From now on I'm going to avoid rebase until I know the code is ready.
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

The rebar_rail ordering was part of the original PR. it can be found at the end of the gun update PR #16863 .
@BorkBorkGoesTheCode

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 28, 2016

@kevingranade are you still online?
@kevingranade

This comment has been minimized.

Sign in to view
Copy link
Member

commented May 29, 2016

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.