Skip to content

Move contraband text to a separate examine tab#32250

Merged
metalgearsloth merged 11 commits intospace-wizards:masterfrom
murphyneko:move-contra-text
Feb 11, 2025
Merged

Move contraband text to a separate examine tab#32250
metalgearsloth merged 11 commits intospace-wizards:masterfrom
murphyneko:move-contra-text

Conversation

@murphyneko
Copy link
Copy Markdown
Contributor

@murphyneko murphyneko commented Sep 17, 2024

i forgor i had this pr open and deleted it whoops

About the PR

Contraband text is now in a separate examination tab, like armor resistances and item damage.

Why / Balance

That info is related to a single department (security), I don't need to see it every time I examine something.
You can see that tab in the strip menu, meaning it's only a visual difference.

Technical details

ContrabandSystem was refactored a bit for that

Media

image
image

Requirements

Breaking changes

Changelog
note that i added changelog after the merge to see if this works; if it does, it can be abused and should be fixed
🆑

  • tweak: Contraband examine is separated into a different section of examine.

@Sadopeer
Copy link
Copy Markdown

this is a brilliant addition

@Everturning
Copy link
Copy Markdown

is the knife a placeholder? might be better to make it something like handcuffs

@murphyneko
Copy link
Copy Markdown
Contributor Author

is the knife a placeholder? might be better to make it something like handcuffs

i am not the author of the icon

Copy link
Copy Markdown
Contributor

@BramvanZijp BramvanZijp 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 great, works in-game.

@github-actions github-actions Bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 23, 2024
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Errant-4 Errant-4 added the S: Undergoing Discussion Status: Currently going through an extended discussion, as per procedure. label Sep 25, 2024
# Conflicts:
#	Content.Shared/Contraband/ContrabandSystem.cs
@github-actions github-actions Bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 25, 2024
@murphyneko
Copy link
Copy Markdown
Contributor Author

@Errant-4 any news on maintainer discussion? AFAIK it's 3 days

@Errant-4
Copy link
Copy Markdown
Member

@Errant-4 any news on maintainer discussion? AFAIK it's 3 days

Discussion ran into a design vacuum, vote has not started yet

@murphyneko
Copy link
Copy Markdown
Contributor Author

g

@Errant-4 Errant-4 self-assigned this Oct 26, 2024
@superjj18

This comment was marked as abuse.

@murphyneko
Copy link
Copy Markdown
Contributor Author

But for this I feel like you're just slowing down the player getting info for very little value.

It's a matter of milliseconds to click a button, plus you're most likely viewing it from the strip menu/from the item on the ground, making speed a non-issue

@SlamBamActionman SlamBamActionman added the S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. label Nov 14, 2024
@beck-thompson beck-thompson added P3: Standard Priority: Default priority for repository items. T: New Feature Type: New feature or content, or extending existing content D3: Low Difficulty: Some codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. A: Roundflow/Antag Area: Roundflow - "What happens in the game", including antagonist roles and their capabilities A: Security Area: Security department, including Detectives, HoS size/M Denotes a PR that changes 100-999 lines. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Nov 21, 2024
@SlamBamActionman
Copy link
Copy Markdown
Member

Hello, sorry for the late reply; internal discussion resulted in feedback but it was never finalized.

The consensus seems to be that the preferable way to implement this kind of examine status would be via a tooltip-on-hover icon, rather than its own clickable button + window. That way you don't have to do a separate click nor move away from the item description. Were we to do this, any such hover-only icon should probably be left-aligned in the window, just to separate it from the clickable buttons.

Speaking of the icon, there was some disagreements about its depiction. A knife is serviceable but maybe doesn't perfectly convey that it's for contraband info. Ubaser suggested a lock, for example. With the hover-only suggestion above it would be great if the icon changed visuals depending on if you're permitted to have the item or not, though I'm not entirely sure we have code supporting that functionality.

All that being said, if you're not interested in creating a hover-only tooltip icon we could merge this as an interim solution since it is an improvement over the current state, though it would require a change from the knife icon first.

@SlamBamActionman SlamBamActionman added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Jan 4, 2025
@github-actions github-actions Bot added S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Jan 11, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions Bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 21, 2025
@murphyneko
Copy link
Copy Markdown
Contributor Author

murphyneko commented Jan 21, 2025

help me

back to fixing

# Conflicts:
#	Content.Shared/Contraband/ContrabandSystem.cs
…valuate the pull request.

This pull request has conflicts, please resolve those before we can evaluate the pull request.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
@github-actions github-actions Bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 22, 2025
@murphyneko
Copy link
Copy Markdown
Contributor Author

if anyone spots this, this is not my fault, it works the same on master (see #34581)
image

@github-actions github-actions Bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 23, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

# Conflicts:
#	Content.Shared/Contraband/ContrabandSystem.cs
#	Resources/Locale/en-US/contraband/contraband-severity.ftl
@github-actions github-actions Bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 23, 2025
@Errant-4 Errant-4 added the S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. label Jan 23, 2025
@github-actions github-actions Bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 24, 2025
@github-actions
Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

# Conflicts:
#	Content.Shared/Contraband/ContrabandSystem.cs
@github-actions github-actions Bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 24, 2025
@metalgearsloth metalgearsloth merged commit a62ffdc into space-wizards:master Feb 11, 2025
@murphyneko murphyneko deleted the move-contra-text branch February 11, 2025 20:33
@murphyneko murphyneko mentioned this pull request Feb 16, 2025
2 tasks
Jakumba pushed a commit to TheDenSS14/TheDen that referenced this pull request Aug 9, 2025
## About the PR
This PR adds the system that's going to allow us to mark items as
contraband.
Adding the respective contraband parent to items is going to be for
another pr.

ported:
- space-wizards/space-station-14#28688
- space-wizards/space-station-14#30986
- space-wizards/space-station-14#30930
- space-wizards/space-station-14#30942
- space-wizards/space-station-14#31606
- space-wizards/space-station-14#30970
- space-wizards/space-station-14#33385
- space-wizards/space-station-14#32250
- space-wizards/space-station-14#35206
- space-wizards/space-station-14#35228
- space-wizards/space-station-14#36032

## Why / Balance
It'll allow people to definitely discern whether an item is contraband
or not and what severity.

## Technical details
Look at PR's mentioned.

## Media
(this pr does not actually change these items)
<img width="628" height="165" alt="image"
src="https://github.com/user-attachments/assets/92c7bb4c-0f8f-46cf-9d7f-c9de29a1a473"
/>
<img width="644" height="268" alt="image"
src="https://github.com/user-attachments/assets/053a211c-8f4c-435c-beff-62e3b84a01fe"
/>
<img width="886" height="170" alt="image"
src="https://github.com/user-attachments/assets/09c661aa-b7f8-4e51-bd3d-9110fe22657b"
/>


## Requirements
- [x] I have read and am following the [Pull Request and Changelog
Guidelines](https://docs.spacestation14.com/en/general-development/codebase-info/pull-request-guidelines.html).
- [x] I have added media to this PR or it does not require an ingame
showcase.
- [x] I can confirm this PR contains no AI-generated content, and did
not use any AI-generated content.

## Breaking changes
None as far as I know.

## Changelog
Not needed.

---------

Co-authored-by: Kara <lunarautomaton6@gmail.com>
Co-authored-by: EmoGarbage404 <retron404@gmail.com>
Co-authored-by: Nemanja <98561806+EmoGarbage404@users.noreply.github.com>
Co-authored-by: Brandon Hu <103440971+Brandon-Huu@users.noreply.github.com>
Co-authored-by: slarticodefast <161409025+slarticodefast@users.noreply.github.com>
Co-authored-by: Plykiya <58439124+Plykiya@users.noreply.github.com>
Co-authored-by: Winkarst <74284083+Winkarst-cpu@users.noreply.github.com>
Co-authored-by: John <35928781+sporkyz@users.noreply.github.com>
Co-authored-by: Ed <96445749+TheShuEd@users.noreply.github.com>
Co-authored-by: Killerqu00 <47712032+Killerqu00@users.noreply.github.com>
eris-webserv pushed a commit to The-Arcadis-Team/arc-station-14 that referenced this pull request Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A: Roundflow/Antag Area: Roundflow - "What happens in the game", including antagonist roles and their capabilities A: Security Area: Security department, including Detectives, HoS D3: Low Difficulty: Some codebase knowledge required. P3: Standard Priority: Default priority for repository items. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. S: Undergoing Discussion Status: Currently going through an extended discussion, as per procedure. size/M Denotes a PR that changes 100-999 lines. T: New Feature Type: New feature or content, or extending existing content

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants