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

Message Player if Inbox is Full #91

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AntumDeluge
Copy link
Contributor

@AntumDeluge AntumDeluge commented Jan 6, 2025

Sends text "This mailbox is full." to player if there is no room if setting homedecor.verbose_messages is enabled.

Original message:

Sends text "The mailbox is full." to player if there is no room when trying to deposit an item.

@OgelGames
Copy link
Contributor

OgelGames commented Jan 6, 2025

I think "This mailbox is full." would be more grammatically correct, because there can be multiple mailboxes.

@SwissalpS
Copy link
Contributor

I don't like chat messages for obvious things.
This PR sends a message to the player putting items into the mailbox. They already see that the inventory is full as their items are being returned to them.
This is the equivelant of sending a success message "Thanks for dropping items". The player already sees that their items have been taken.

It would be another matter, if the owner is getting a message (once, we don't want to create an opportunity for bad actors to spam the owner).
However that would require checking if the owner is online and opens the question about informing them when they join.

An alternative could be to send the owner a /mail "Your mailbox at x, y, z is full".

@BuckarooBanzay BuckarooBanzay added the enhancement New feature or request label Jan 9, 2025
Copy link
Member

@BuckarooBanzay BuckarooBanzay left a comment

Choose a reason for hiding this comment

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

code looks good, @SwissalpS has a point though, do the returning items really go unnoticed?

@AntumDeluge
Copy link
Contributor Author

AntumDeluge commented Jan 18, 2025

I might agree if the player could see that the mailbox inventory is full or see the result of the drop (e.g. an item dropped on the ground can visibly be seen on the ground). Without inherent knowledge that a mailbox has a capacity limit, players might not understand why their item isn't accepted & may confuse it with a server error.

Perhaps to find a middle ground the notification can be enabled with a setting while being disabled by default.

Edit: For context, my server is used primarily by younger children who often need feedback because they don't understand why something might not work.

@AntumDeluge
Copy link
Contributor Author

I think "This mailbox is full." would be more grammatically correct, because there can be multiple mailboxes.

Changed.

@SwissalpS
Copy link
Contributor

While I still think it isn't needed, I'm OK with this variant.

@wsor4035
Copy link
Collaborator

naming the setting homedecor.verbose_messages seems rather wide scope, when its just for mailboxes

@SwissalpS
Copy link
Contributor

SwissalpS commented Jan 19, 2025

maybe they want to add more messages like: this refrigerator is empty/full etc. :D

Edit: at which point it could just as well be called "homedecor.chat_debugging"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants