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

Breaking up some of lefthand.dmi and righthand.dmi. #3316

Merged
merged 9 commits into from
Oct 27, 2023

Conversation

MistakeNot4892
Copy link
Contributor

Description of changes

Deletes several unused states from lefthand/righthand.dmi.
Reworks various items to use the single icon system instead.

Why and what will this PR improve

Someday I want to destroy righthand.dmi and lefthand.dmi and this is needed for that.

Authorship

Myself.

Changelog

Nothing player-facing.

@MistakeNot4892 MistakeNot4892 added work in progress This PR is under development and shouldn't be merged. ready for review This PR is ready for review and merge. and removed work in progress This PR is under development and shouldn't be merged. labels Sep 12, 2023
Copy link
Member

@out-of-phaze out-of-phaze left a comment

Choose a reason for hiding this comment

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

Did you make sure to test that syringe filling overlays still work? Without IDB, it's hard to check that without checking out the branch and running it, which is kind of a pain.

@@ -17,8 +17,10 @@
. = ..()
underlays.Cut()
if(syringe)
underlays += image(syringe.icon, src, syringe.icon_state)
underlays += syringe.filling
var/mutable_appearance/MA = syringe.appearance
Copy link
Member

Choose a reason for hiding this comment

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

Are pixel_x and pixel_y cleared? Would they even apply anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Member

@out-of-phaze out-of-phaze Oct 23, 2023

Choose a reason for hiding this comment

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

	if(randpixel)
		pixel_z = randpixel //an idea borrowed from some of the older pixel_y randomizations. Intended to make items appear to drop at a character

Looks like pixel_z should also be cleared. :(
EDIT: Maybe pixel_w too, at this point.

icon_state = "hypo"
icon = 'icons/obj/hypospray.dmi'
icon_state = ICON_STATE_WORLD
abstract_type = /obj/item/chems/hypospray
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it convention for abstract_type to come first, i.e. before name =? That makes it clearer that it's abstract.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't aware we had a convention for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per Discord, leaving it where it is, but I readded a comment.

@MistakeNot4892 MistakeNot4892 changed the title Breaking up some of lefthand.dmi and righthand.dmi. [IDB IGNORE] Breaking up some of lefthand.dmi and righthand.dmi. Oct 20, 2023
@@ -17,8 +17,10 @@
. = ..()
underlays.Cut()
if(syringe)
underlays += image(syringe.icon, src, syringe.icon_state)
underlays += syringe.filling
var/mutable_appearance/MA = syringe.appearance
Copy link
Member

@out-of-phaze out-of-phaze Oct 23, 2023

Choose a reason for hiding this comment

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

	if(randpixel)
		pixel_z = randpixel //an idea borrowed from some of the older pixel_y randomizations. Intended to make items appear to drop at a character

Looks like pixel_z should also be cleared. :(
EDIT: Maybe pixel_w too, at this point.

@out-of-phaze out-of-phaze added awaiting author This PR is awaiting action from the author before it can be merged. and removed ready for review This PR is ready for review and merge. labels Oct 23, 2023
@MistakeNot4892 MistakeNot4892 added ready for review This PR is ready for review and merge. and removed awaiting author This PR is awaiting action from the author before it can be merged. labels Oct 25, 2023
@out-of-phaze out-of-phaze merged commit e54f278 into NebulaSS13:dev Oct 27, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review This PR is ready for review and merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants