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

fixes xenos seeing raw html on paper #4654

Closed
wants to merge 5 commits into from
Closed

fixes xenos seeing raw html on paper #4654

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 12, 2023

About the pull request

currently work in progress, fixes the bug as outlined here #4609

Explain why it's good for the game

bug bad

Testing Photographs and Procedure

xeno_paper_bug.mp4

Changelog

🆑
fix: xenos viewing paper no longer displays unformatted raw HTML
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Oct 12, 2023
Copy link
Member

@harryob harryob left a comment

Choose a reason for hiding this comment

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

nit picks

code/modules/mob/mob_helpers.dm Outdated Show resolved Hide resolved
code/modules/mob/mob_helpers.dm Outdated Show resolved Hide resolved
code/modules/mob/mob_helpers.dm Outdated Show resolved Hide resolved
@ghost ghost marked this pull request as draft October 13, 2023 19:06
Cthulhu80 and others added 2 commits October 13, 2023 15:53
Copy link
Contributor

@DoomLaser9 DoomLaser9 left a comment

Choose a reason for hiding this comment

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

Two things: HTML attribute handling and replacetext issues

Comment on lines +150 to +154
if(message[character_index] == ">")
parsing_message = TRUE
continue
if(message[character_index] == "<")
parsing_message = FALSE
Copy link
Contributor

Choose a reason for hiding this comment

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

You also need to check for quotation marks inside of HTML tags. Consider a tag such as <tag property=">Injection()" />. So when you are inside an HTML tag and run into a quotation mark (either single ' or double "), then you advance until you find the matching quotation mark. Perhaps you could add a couple booleans for this, e.g. need_single_quote and need_double_quote.

Maybe it would be a bit overkill but it's probably good to be safe.

Copy link
Author

Choose a reason for hiding this comment

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

Alright.

current_string_to_scramble += message[character_index]

for(var/unscrambled_message_key in replaced_scrambled_strings)
message = replacetext(message, unscrambled_message_key, replaced_scrambled_strings[unscrambled_message_key])
Copy link
Contributor

Choose a reason for hiding this comment

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

Carefully-crafted user input would make this replace text inside of HTML tags. This should be reworked to be a bit more similar to how /proc/stars works, where the final message is being appended to in the original for loop. In this case, you could be appending scrambled_string from the loop before to a variable representing the final output. I suspect that would also be better performance-wise compared to repeatedly searching through the whole text for replacements.

Your current code also means that duplicates of unscrambled_message_key would get the same star text, while doing it iteratively would preserve the original behavior of not duplicating star text instances.

Copy link
Author

@ghost ghost Dec 28, 2023

Choose a reason for hiding this comment

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

I'll make it iterative.

@ghost
Copy link
Author

ghost commented Oct 16, 2023

going to be busy, i'm closing this pr for now. I'll get to it later.

@ghost ghost closed this Oct 16, 2023
@ghost ghost reopened this Dec 28, 2023
@ghost ghost closed this Dec 28, 2023
@ghost ghost reopened this Dec 28, 2023
@ghost
Copy link
Author

ghost commented Dec 28, 2023

branch being real funky, closing and making a new branch.

@ghost ghost closed this Dec 28, 2023
@ghost ghost mentioned this pull request Dec 28, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 31, 2023
# About the pull request
Continuation of #4654 , to fix #4609 . This fix should make it so that
when xenos examine paper it no longer displays a blank page and the raw
html was replaced with the formatted scrambled message.

# Explain why it's good for the game

raw html is rather ugly




https://github.com/cmss13-devs/cmss13/assets/122310258/c279d8c8-e81e-477f-bcd0-cadc2bd32b4e



# Changelog

:cl:
fix: fixes xenos being able to view raw html on paper.
/:cl:

---------

Co-authored-by: harryob <[email protected]>
Co-authored-by: Drathek <[email protected]>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Fix one bug, make ten more
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants