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

Fixed the Riot foam icon #5676

Merged
merged 7 commits into from Sep 15, 2016
Merged

Fixed the Riot foam icon #5676

merged 7 commits into from Sep 15, 2016

Conversation

shazbot194
Copy link
Contributor

@shazbot194 shazbot194 commented Sep 13, 2016

Adds the variable riot to both normal foam darts and adds two else if statements and modifies one if statement to give both darts and both modified darts the correct icon.

Adds the varable riot to both normal foam darts and adds some else if
statments to give both darts and modified darts the correct icon.
@Fox-McCloud
Copy link
Member

This is all...super unnecessary....can be simplified to something much better and future-proof.

icon_state = "foamdart_riot"
desc = "Whose smart idea was it to use toys as crowd control? Ages 18 and up."
if(BB)
BB.icon_state = "foamdart_riot"
else
icon_state = "foamdart"
Copy link
Member

Choose a reason for hiding this comment

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

icon_state = initial(icon_state)

@shazbot194
Copy link
Contributor Author

shazbot194 commented Sep 13, 2016

How about replacing the if, else if, and else block with this;
/obj/item/ammo_casing/caseless/foam_dart/update_icon()
..()
if(modified)
icon_state = "foamdart_empty"
if(riot)
desc = "Whose smart idea was it to use toys as crowd control? ... Although, this one doesn't look too safe."
else
desc = "Its nerf or nothing! ... Although, this one doesn't look too safe."
if(BB)
BB.icon_state = "foamdart_empty"
else
icon_state = initial(icon_state)
if(BB)
BB.icon_state = initial(BB.icon_state)

It is smaller then the one that is currently on the server, slightly more future proof then that one as well and has no bugs, and uses all the proper sprites.
The value "riot" could also be changed to "type" and then each new custom dart could have a number, ie normal = 0, riot = 1, new dart = 2, and so on.

@Fox-McCloud
Copy link
Member

If you implemented what I stated, it's completely future proof for other colored darts, as long as the icon state on the foam dart and casing are properly set on the subtype.

@shazbot194
Copy link
Contributor Author

shazbot194 commented Sep 13, 2016

Never mind, I found out how to edit the Pull.

@Fox-McCloud
Copy link
Member

Are you...using the git text editor?

@shazbot194
Copy link
Contributor Author

I am using dream maker but I just found the Git editor, I did just test this code however and it does work.

if(riot)
desc = "Whose smart idea was it to use toys as crowd control? ... Although, this one doesn't look too safe."
else
desc = "Its nerf or nothing! ... Although, this one doesn't look too safe."
Copy link
Member

Choose a reason for hiding this comment

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

The above reallyyyy isn't necessary.

@shazbot194
Copy link
Contributor Author

I agree, I was trying to keep in how the dart's description says it looks unsafe, but without it does cut down on any additional lines if someone wants to add more darts in, shall I just take it out?

@Fox-McCloud
Copy link
Member

Yeah, I'd just...take it out to be honest--the major problem here are the icons messing up.

@shazbot194
Copy link
Contributor Author

Ok, I took out the changing description and just left what is needed for the icons.

@@ -306,19 +306,18 @@
icon = 'icons/obj/guns/toy.dmi'
icon_state = "foamdart"
var/modified = 0
var/riot = 0
Copy link
Member

Choose a reason for hiding this comment

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

unused var.

@shazbot194
Copy link
Contributor Author

shazbot194 commented Sep 13, 2016

How about making it something along the lines of "This foam dart doesn't look too safe" so its more generic.

The reason for a slightly more generic description is to help reduce the odds of a riot/new dart being confused for a normal foam force but still lets people know that is doesn't have the safety cap on.

Also I took out the val/riot since it is no longer needed.

@shazbot194
Copy link
Contributor Author

Well I think its just about as future proof as it can get right now, although I doubt its perfect, any more thoughts? I have also tested it, all the icons are now displayed correctly.

changes how the stechin updates it's magazine so the single shot
magazine doesn't go invisable.
@marlyn-x86
Copy link
Member

marlyn-x86 commented Sep 14, 2016

update ammo_casings.dm 👀

Fee, fi, foo, fur,
I spot the mark of the github editor!

@shazbot194
Copy link
Contributor Author

uhhh, that last bit for the stechin pistol isn't meant to be on this one, oops.

@shazbot194
Copy link
Contributor Author

shazbot194 commented Sep 14, 2016

yup, I wasn't sure how to update the pull via dream maker but it looks like I did by accident.

@Fox-McCloud
Copy link
Member

I've said it once, but it looks like I'm going to to have to say it again. If you can't be bothered to set up git and make changes in DM, directly, then don't bother submitting a PR at all. Github editor submitted PRs will be denied.

@PJB3005
Copy link
Contributor

PJB3005 commented Sep 14, 2016

@Fox-McCloud and, how does that matter in any way? Code is code.

@Fox-McCloud
Copy link
Member

Because it means they didn't bother compiling or testing it.

@shazbot194
Copy link
Contributor Author

shazbot194 commented Sep 14, 2016

I did compile it and test it, I even said I wasn't sure how to update if from dream maker until I did it by accident. In addition to the multiple times I said I have tested it along the way, if you would like I could republish it from dream maker, but it would be the same exact tested code.

icon_state = "foamdart_empty"
desc = "Its nerf or nothing! ... Although, this one doesn't look too safe."
Copy link
Member

Choose a reason for hiding this comment

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

Don't change the description or order unless there's an explicit reason to. In this case, there isn't.

@Fox-McCloud Fox-McCloud added the Fix This PR will fix an issue in the game label Sep 15, 2016
changes the discription and moves it one line down.
@shazbot194
Copy link
Contributor Author

I still think that a the description "Its nerf or nothing! ... Although, this one doesn't look too safe." doesn't fit too well on modified riot darts and the slightly more generic "This foam dart doesn't look too safe" works well for both uncapped foam force, riot darts, and any future darts, but here is it is with the former description.

@Fox-McCloud Fox-McCloud merged commit da6a80c into ParadiseSS13:master Sep 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix This PR will fix an issue in the game
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants