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

Unifies and Overhauls round start/late join/ERT/Predalien text. #5048

Closed
wants to merge 8 commits into from
Closed

Unifies and Overhauls round start/late join/ERT/Predalien text. #5048

wants to merge 8 commits into from

Conversation

silencer-pl
Copy link
Contributor

@silencer-pl silencer-pl commented Nov 27, 2023

About the pull request

This PR mostly unifies (more below) how roundstart/ert/late join and predalien for some reason text is generated and presented. Gone are the 4 or so different spans used fairly randomly across the board, replaced by two style sheets - role_header and role_body

It also removes the "force captainship" mechanic that CM does not use and seems to exist mostly to indicate that there is no CO which is obvious because CO gets their own global entry message. The text also still indicated that captainship can be somehow forced and while that is somewhat true, its an IC Chain of Command mechanic.

Finally, it removes a legacy tgui ticker feature where the role name, decapitalized, would be printed at the top of the blurb. CM standard already prints the role name in the header, this text was redundant and looked weird without capitals, especially if say someone downstream used a six letter acronym for a role name.

The advantages of moving out of spans and into padded divs should be obvious after looking at samples in the screenshot section. Here is a small list anyway:

  • The text now respects spacing and sizing preferences from tgui chat.
  • Instead of relaying on breakpoints and lines of underscores, it uses padded divs and paragraphs.
  • This means that text in the body can be properly justified. Someone tried to justify by hand in one of the ert descriptions, the poor soul.

This also cleans up the lists (???) that survivors were used to generate their spawn in text, which for some reason uses a completely different method to store and display them. I do not have the energy required to rewrite this system to 100% match the other roles, instead I split the lists into two vars and printed one as the header and the other as the body, which is way more like how this is set up for the other roles.

Finally, since it was using the same classes, this also makes the predalien text follow the same rules. It even has a big text telling them what to do.

No text was actually changed, I only edited formatting.

THIS WILL LIKELY REQUIRE A TM to make sure that the survivor and upp/clf survivor text displays properly. Everything else should be fine.

To completly remove the role header and body spans, I also changed how the fun facts at the end of the round are displayed, but the impact should be minimal. If needs be I can restyle that too.

If this goes thorough I can likely make xeno spawn in text follow the same rules for consistency's sake. Regardless, I'll be using this formatting in my downstream so if this is not something you're into, no hard feelings. If you however are, I also have admin narrate changes that make it stand out more, but later etc etc.

YES, I PUT paragraph openers and closers at the macro defines, trust me, after 100+ paragraphs put into datums, its the better way.

Explain why it's good for the game

I'm not going to lie, this heavily benefits my text heavy downstream but I hope that as seen on the presented screenshots, this genuinely is a vastly more attractive way of displaying flavor text.

Since this text scales to tgui settings, its already way more likely to be exactly how the potential reader wants to see it. The slight padding and better centering makes the text stand out without the need of underscore lines.

Finally, justifying the large paragraphs of text give the whole thing a professional feel. Again, I think the screenshots speak for themselves.

Testing Photographs and Procedure

Screenshots & Videos

My settings for reference:
image
CO ROUNDSTART:
image
RIFLEMAN LATEJOIN:
image
The dot after the squad name was already fixed thanks to this screenshot :P
ERT LATEJOIN:
image
My humble suggestion: Yeet the objectives and objectives printout into the sea. Replace it with another paragraph or something. Anyway, that's not really here or there.

Changelog

🆑silencer_pl
ui: Overhauled and unified how round start, late join, ERT late join and predalien role description text is displayed.
/:cl:

@github-actions github-actions bot added the UI deletes nanoui/html label Nov 27, 2023
Copy link
Member

@fira fira left a comment

Choose a reason for hiding this comment

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

There are stray

in the middle of some role_body which partly defeat the point of having the define encapsultating the block
Is that just meant to be spacing? You could try using simply
instead

@silencer-pl
Copy link
Contributor Author

silencer-pl commented Nov 27, 2023

Fun fact: Github comments eat html tags

Pretty sure I intended to find and replace all strays at some point and may have forgetten, I'll take a look in a few.

@silencer-pl
Copy link
Contributor Author

All stray paragraph pairs have been removed, they were either unnecessary or split into headers and bodies. Also made all erts follow the same method cryorines do when it comes to delaying displaying objectives, slightly clanging how the objectives readout looks. Updated screenshot posted in description.

@silencer-pl
Copy link
Contributor Author

There are stray

in the middle of some role_body which partly defeat the point of having the define encapsultating the block Is that just meant to be spacing? You could try using simply instead

They were indeed meant to be spacing and the paragraphs were more or less intended, but I removed all the doubles etc and made it proper paragraphs so there is clarity, should be good on that regard.

@cm13-github
Copy link
Contributor

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

@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Dec 1, 2023
@cm13-github cm13-github removed the Merge Conflict PR can't be merged because it touched too much code label Dec 2, 2023
@cm13-github
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@fira fira added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Dec 3, 2023
@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Dec 4, 2023
@cm13-github
Copy link
Contributor

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

@cm13-github cm13-github removed the Merge Conflict PR can't be merged because it touched too much code label Dec 4, 2023
@cm13-github
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

I'm surprised Fira is okay with the removal of timers in favor of sleeps; but a lot of this code is already using lots of sleeps and spawn.

[SPAN_ROLE_BODY("|______________________|")] \
"
to_chat_spaced(H, html = entrydisplay)
var/entrydisplay = "[generate_entry_message(H)][M ? role_body("Your account number is: <b>[M.account_number]</b>. Your account pin is: <b>[M.remote_access_pin]</b>.") : role_body("You do not have a bank account.")][flags_startup_parameters & ROLE_ADMIN_NOTIFY ? role_header("<big>You are playing a job that is important for game progression. If you have to disconnect, please notify the admins via adminhelp.</big>") : ""]"
Copy link
Contributor

Choose a reason for hiding this comment

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

This really needs to be broken up into multiple variables that you can concatenate together in the following to_chat call. E.g. var/account and var/important and then the to_chat has [generate_entry_message(H)][account][important] If there's no account then leave account as ""; same for important.

It used to be clear what was on each line, now its just a giant line of text spanning over 400 characters (used to be at worst ~200 characters) in code with multiple large ternary conditional operators.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that whole message is fairly ugly as is. I'll take a look at it when I have a moment and see what I can come up with.

@silencer-pl
Copy link
Contributor Author

Closing. Suggested changes are out of scope for my downstream as I'll likely dump the randomly generated bank completly and I at this point contributing my time to resolving this is putting hours more than I originally intended to spend porting this upstream in the first place. Please feel free to use my diffs to reopen this in the future.

@Drulikar
Copy link
Contributor

I'm not sure how changing

var/entrydisplay = "[generate_entry_message(H)][M ? role_body("Your account number is: <b>[M.account_number]</b>. Your account pin is: <b>[M.remote_access_pin]</b>.") : role_body("You do not have a bank account.")][flags_startup_parameters & ROLE_ADMIN_NOTIFY ? role_header("<big>You are playing a job that is important for game progression. If you have to disconnect, please notify the admins via adminhelp.</big>") : ""]"
to_chat(H, html = entrydisplay)

to

var/acount = M ? role_body("Your account number is: <b>[M.account_number]</b>. Your account pin is: <b>[M.remote_access_pin]</b>.") : role_body("You do not have a bank account.")
var/admin_notify = (flags_startup_parameters & ROLE_ADMIN_NOTIFY) ? role_header("<big>You are playing a job that is important for game progression. If you have to disconnect, please notify the admins via adminhelp.</big>") : ""
to_chat(H, html = "[generate_entry_message(H)][account][admin_notify]")

Is hours worth of work but alright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testmerge Candidate we'll test this while you're asleep and the server has 10 players UI deletes nanoui/html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants