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

Package Delivery Expansion #20003

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Geevies
Copy link
Contributor

@Geevies Geevies commented Oct 5, 2024

  • Cargo delivery packages now have colored tags on them which should hopefully make it easier to distinguish between them, instead of changing the entire package's color.
  • Delivering a package now outputs another, allowing you to continuously deliver throughout the round, should you choose to.
  • Fixed cargo packages runtiming on the runtime map.

@BotBOREALIS BotBOREALIS added the Sprites Adds new or changes existing sprites. label Oct 5, 2024
@github-actions github-actions bot added 🗺️ Mapping - Runtime The PR touches the Runtime map files. 🗺️ Mapping - Horizon The PR touches the Horizon map files. labels Oct 5, 2024
@Geevies
Copy link
Contributor Author

Geevies commented Oct 5, 2024

!review

@@ -41,7 +44,7 @@
pay_amount = rand(12, 17) * 1000
if(delivery_point)
setup_delivery_point(delivery_point)
color = pick("#FFFFFF", "#EEEEEE", "#DDDDDD", "#CCCCCC", "#BBBBBB", "#FFDDDD", "#DDDDFF", "#FFFFDD", "#886600")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should stay
The colors here are like off-white shades of gray-ish
I think it makes sense that the courier packages aren't all perfectly shiny and white and clean
Cause they are used all over the spur and can come from anywhere

Comment on lines +79 to +84
var/package_index = 1
for(var/obj/item/cargo_package/package in contained_packages)
var/image/package_tag = image(icon, icon_state = "package_pack_[package_index]_tag")
package_tag.color = package.accent_color
mob_overlay.AddOverlays(package_tag)
package_index++
Copy link
Contributor

@FluffyGhoster FluffyGhoster Oct 10, 2024

Choose a reason for hiding this comment

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

Suggested change
var/package_index = 1
for(var/obj/item/cargo_package/package in contained_packages)
var/image/package_tag = image(icon, icon_state = "package_pack_[package_index]_tag")
package_tag.color = package.accent_color
mob_overlay.AddOverlays(package_tag)
package_index++
for(var/package_index in 1 to lenght(contained_packages))
var/obj/item/cargo_package/package = contained_packages[package_index]
var/image/package_tag = image(icon, icon_state = "package_pack_[package_index]_tag")
package_tag.color = package.accent_color
mob_overlay.AddOverlays(package_tag)

Something like this is clearer

var/obj/structure/cargo_receptacle/selected_delivery_point = get_cargo_package_delivery_point(src)
if(selected_delivery_point)
visible_message("\The [src] beeps, \"[SPAN_NOTICE("New package available for delivery.")]\"")
playsound(loc, /singleton/sound_category/print_sound, 50, TRUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
playsound(loc, /singleton/sound_category/print_sound, 50, TRUE)
playsound(src, /singleton/sound_category/print_sound, 50, TRUE)

There shouldn't be a need to use loc here, since it's a structure?

@NonQueueingMatt NonQueueingMatt added the Changes Required The PR requires changes before it can be approved and/or merged. label Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Required The PR requires changes before it can be approved and/or merged. 🗺️ Mapping - Horizon The PR touches the Horizon map files. 🗺️ Mapping - Runtime The PR touches the Runtime map files. Review Required Sprites Adds new or changes existing sprites.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants