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

Crew Credits Antagonists Tab #16185

Merged
merged 10 commits into from
Oct 8, 2023

Conversation

Mister-Moriarty
Copy link
Contributor

@Mister-Moriarty Mister-Moriarty commented Oct 1, 2023

[UI] [QoL]

About The PR:

Implements an antagonists tab to the end of round crew credits, displaying details regarding each major antagonist of the round, including their antagonist roles; job; status; objectives; general statistics, such as purchased equipment or blood drank; and their subordinate antagonists.

Antagonists not considered major, such as subordinate antagonists, generic antagonists, or revolutionaries will be displayed under the "Other Antagonists" section. If a subordinate antagonist has a master, they will instead be listed under their master's data entry.

image

Additionally to better facilitate a tab oriented structure, the crew credits datum has been refactored.

Why Is This Needed?

Deprecates the old text window readout of antagonist statistics, and therefore fixes #15870.

Changelog:

(u)Mr. Moriarty
(*)Added an antagonists tab to the end of round crew credits.

@keywordlabeler keywordlabeler bot added A-UI Modifies UI in some way. Automatically applied on a change to tgui/ C-Bug A bug that impacts usage of a feature C-QoL A quality of life improvement that makes the game easier to play labels Oct 1, 2023
@github-actions github-actions bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict labels Oct 1, 2023
@444Portal
Copy link
Contributor

Hell yeah! Looks great.

@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Oct 1, 2023
tgui/packages/tgui/components/goonstation/ItemList.tsx Outdated Show resolved Hide resolved
tgui/packages/tgui/components/goonstation/ItemList.tsx Outdated Show resolved Hide resolved
tgui/packages/tgui/components/goonstation/ItemList.tsx Outdated Show resolved Hide resolved
Comment on lines 22 to 42
<Flex
inline
align="center"
key={index}>
{!!item.icon && (
<Flex.Item>
<Image
height="32px"
width="32px"
pixelated
src={`data:image/png;base64,${item.icon}`}
/>
</Flex.Item>
)}
<Flex.Item
pr={1}
pl={0.5}>
{item.name}
{`${index === arr.length - 1 ? '' : `, ${index === arr.length - 2 ? 'and ' : ''}`}`}
</Flex.Item>
</Flex>
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than setting pr, pl etc., does this work if you use a Stack instead of Flex? Unsure how that spacing would look here.

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'm using Flex so to avoid spacing other than where explicitly stated.

tgui/packages/tgui/components/goonstation/ItemList.tsx Outdated Show resolved Hide resolved

import { Flex, Image } from '..';

export const ItemList = (props: ItemListProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Broadly can't see much use for this yet outside of CrewCredits. Probably put in there rather than in the common spot, and if someone wants it they can pull it out and change the import, it's just one line.

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 would rather have ItemList be located in the common components list; in order to pull it out of the crew credits folder, and individual first has to know it is located there, whereas when in the common list, it is more visible to UI coders, and as a result is more likely to be used.

tgui/packages/tgui/interfaces/CrewCredits/CrewTab.tsx Outdated Show resolved Hide resolved
@Mister-Moriarty
Copy link
Contributor Author

Mister-Moriarty commented Oct 1, 2023

Thank you kindly for the UI review.
Suggestions have been implemented other than where stated.

@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Oct 2, 2023
Copy link
Contributor

@TobleroneSwordfish TobleroneSwordfish left a comment

Choose a reason for hiding this comment

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

Overall looks great! Thanks for doing this, have some nitpicks:

code/datums/crew_credits/crew_credits.dm Outdated Show resolved Hide resolved
code/datums/crew_credits/crew_credits.dm Outdated Show resolved Hide resolved
@@ -26,6 +26,8 @@ datum/mind

/// A list of every antagonist datum that we have.
var/list/datum/antagonist/antagonists = list()
/// A list of every antagonist datum subordinate to this mind.
var/list/datum/antagonist/subordinate/subordinate_antagonists = list()
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be stored on the antag datum, or are subordinate antags technically linked to the mind instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Subordinate antagonists track their master's mind directly.

code/modules/antagonists/blob/blob.dm Outdated Show resolved Hide resolved
code/modules/antagonists/__antagonist.dm Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Oct 4, 2023
@TobleroneSwordfish TobleroneSwordfish added the S-Testmerged [Dev Only] Testmerged for extended testing (applied by bot) label Oct 4, 2023
@ZeWaka ZeWaka merged commit 247c8cb into goonstation:master Oct 8, 2023
21 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 8, 2023
@Mister-Moriarty Mister-Moriarty deleted the antagonist-credits branch March 3, 2024 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Modifies UI in some way. Automatically applied on a change to tgui/ C-Bug A bug that impacts usage of a feature C-QoL A quality of life improvement that makes the game easier to play S-Testmerged [Dev Only] Testmerged for extended testing (applied by bot) size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
Development

Successfully merging this pull request may close these issues.

Thralls don't show up as antags in the credits
8 participants