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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Links Security Status to Sechuds #8467

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

Conversation

thedarkcrusader
Copy link
Contributor

@thedarkcrusader thedarkcrusader commented Apr 3, 2024

About The Pull Request

This PR links the security record. field from crew records to sechuds allowing them to be seen by those wearing sechuds

Why It's Good For The Game

Allows for much Better in field keeping up to date with incidents. and problems without having to run to a computer every
image

single time just to see a person's history.

Testing

spawned in as operative.
edited secrecords from console
viewed myself with sechuds
Records appeared as normal

Changelog

馃啈
tweak: sechuds now draw their information from the crew record program.
/:cl:

Links sec records to sechuds
@thevandie
Copy link
Contributor

Failed integration test, L bozo

@thedarkcrusader
Copy link
Contributor Author

Contributor

Intergration failed?? why don't you go inter into some bitches homes.

@thevandie
Copy link
Contributor

Intergration failed?? why don't you go inter into some bitches homes.

I am not a crook.

Copy link
Contributor

@Firefox13 Firefox13 left a comment

Choose a reason for hiding this comment

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

Records appeared as normal

How does it work with old chat (believe it or not, it needs to work with old chat), how it works if record contain Details (stuff you add in character setup)?

@thedarkcrusader
Copy link
Contributor Author

thedarkcrusader commented Apr 3, 2024

Records appeared as normal

How does it work with old chat (believe it or not, it needs to work with old chat), how it works if record contain Details (stuff you add in character setup)?

Anything added to security status in setup will appear let me check it in old chat rq tho should still be fine

@thedarkcrusader
Copy link
Contributor Author

Records appeared as normal

How does it work with old chat (believe it or not, it needs to work with old chat), how it works if record contain Details (stuff you add in character setup)?

still works with Old chat but the some of the HTML code like boxes don't show up properly. otherwise works as normal

@Firefox13
Copy link
Contributor

Screenshots please?

@thedarkcrusader
Copy link
Contributor Author

Screenshots please?

image

Copy link
Contributor

@SirRichardFrancis SirRichardFrancis left a comment

Choose a reason for hiding this comment

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

image
This is no bueno, spacings are so massive that half the chat is blank.
Even if some information is meant to be there, text should be compressed for cases when there isn't any.

code/modules/mob/living/carbon/human/human.dm Outdated Show resolved Hide resolved
code/modules/mob/living/carbon/human/human.dm Outdated Show resolved Hide resolved
@thedarkcrusader
Copy link
Contributor Author

image This is no bueno, spacings are so massive that half the chat is blank. Even if some information is meant to be there, text should be compressed for cases when there isn't any.

Those spaces are from my HTML layout

@SirRichardFrancis
Copy link
Contributor

Those spaces are from my HTML layout

So? Make them disappear.

@thedarkcrusader
Copy link
Contributor Author

thedarkcrusader commented Apr 4, 2024

Those spaces are from my HTML layout

So? Make them disappear.

Allright it's gone poof

@thedarkcrusader
Copy link
Contributor Author

Poo

image This is no bueno, spacings are so massive that half the chat is blank. Even if some information is meant to be there, text should be compressed for cases when there isn't any.

On a more serious note would you by. Chance know how one could do that compression??

Co-authored-by: SirRichardFrancis <65828539+SirRichardFrancis@users.noreply.github.com>
@SirRichardFrancis
Copy link
Contributor

On a more serious note would you by. Chance know how one could do that compression??

As a matter of fact, I can't find the code responsible for this, since text from your screenshots does not appear to be in the build, and neither is added by this PR.
You're sure that no changes remain uncommitted?

@thedarkcrusader
Copy link
Contributor Author

On a more serious note would you by. Chance know how one could do that compression??

As a matter of fact, I can't find the code responsible for this, since text from your screenshots does not appear to be in the build, and neither is added by this PR. You're sure that no changes remain uncommitted?

Im positive which text are you referring to ?

@SirRichardFrancis
Copy link
Contributor

Im positive which text are you referring to ?

Anything from the screenshot you put in PR description.
image

For example let's take last table header.
image

Or some of the blue text.
image

Other word perhaps?
image

There could be all sorts of weird formatting in strings, which could make them borderline unsearchable, but individual words always fair game. Yet there isn't any in the code.
This table and text around it doesn't seem to exist anywhere.
So I have absolutely no clue where this table is coming from, as such can't help with formatting it properly.

@SirRichardFrancis
Copy link
Contributor

If you made that table in your character's records, then this is nonsensical feature presentation.

@thedarkcrusader
Copy link
Contributor Author

thedarkcrusader commented Apr 4, 2024

If you made that table in your character's records, then this is nonsensical feature presentation.

I mean how would you expect me to show it off this is quite literally one of the end results of editing the security records.

@thedarkcrusader
Copy link
Contributor Author

,
image

If you made that table in your character's records, then this is nonsensical feature presentation.

@SirRichardFrancis
Copy link
Contributor

I mean how would you expect me to show it off this is quite literally one of the things people are able to do now

Using something realistic, like what an actual player, that seen the sun at least once in past three years, could write?
PR description made me think that every sechud examine will have that cursed HTML table.
Since that's apparently just a skill issue on the player part, I have no objections.

@MLGTASTICa
Copy link
Contributor

Note for maintainers , someone should really check if sec records are actually sanitized in the field that this PR outputs , else someone could inject JS.

thedarkcrusader and others added 2 commits April 4, 2024 09:39
Co-authored-by: SirRichardFrancis <65828539+SirRichardFrancis@users.noreply.github.com>
read var deleted
@github-actions github-actions bot added the Merge Conflict Merge Conflict label Apr 12, 2024
@thedarkcrusader
Copy link
Contributor Author

why is this conflicting?

@SirRichardFrancis
Copy link
Contributor

why is this conflicting?

Because you and someone else (who got their PR merged first) have edited the same file.
Just pull upstream/master on your local branch, solve the conflict in VSC, save file, git add, git commit, and git push.
5 minutes adventure.
image

@thedarkcrusader
Copy link
Contributor Author

why is this conflicting?

Because you and someone else (who got their PR merged first) have edited the same file. Just pull upstream/master on your local branch, solve the conflict in VSC, save file, git add, git commit, and git push. 5 minutes adventure. image

Ahh jeez rick i don't want to go on a 5 minute adventure

@Humonitarian
Copy link
Contributor

Merge conflicts still present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge Conflict Merge Conflict
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants