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

Papercode Optimization and Fixes #9410

Merged
merged 5 commits into from
Jul 21, 2023

Conversation

itsmeow
Copy link
Member

@itsmeow itsmeow commented Jul 15, 2023

About The Pull Request

Ports:

Partial (tgui proc):

Why It's Good For The Game

Paper being writeable and not lagging your client to death is nice.

Testing Photographs and Procedure

Screenshots&Videos

Stamp sound yayy
image

This super giant paper was not laggy at all. Before I was getting lag over 400 characters.
image

Copied paper field
image
image

Counting characters properly
image

Changelog

🆑
add: Added a stamp sound effect to papers.
fix: Fixed paper lagging the client when any large amount of characters were written.
fix: Fix photocopied paper input fields not working.
fix: Fixed paper counting cyrillic and other unicode characters as multiple characters.
/:cl:

RedBaronFlyer and others added 5 commits July 14, 2023 20:02
This pull request adds a .ogg file of a stamp stamping a paper, a
playsound function for stamping a paper, stamping a paper while blind,
and stamping yourself in the head to commit suicide.

In addition, it modifies the attributions.txt file to include credits to
the creator of the stamp noise.

Most item interactions tend to have a sound effect that plays. Putting
most items down, throwing them, cutting certain items with the knife,
scanning bounty cubes, putting a destination tag on a package, etc. all
play sounds, which makes it a bit odd that stamping papers doesn't. This
PR adds audio feedback to stamping papers.

https://user-images.githubusercontent.com/45489195/210032424-d3168a16-3d4b-47d3-95f3-75a1d3a6aac9.mp4

Stamping paper now makes noise.

:cl:
soundadd: Added a new sound for stamping papers with a stamp. On an
unrelated note, there are reports of stamp-based suicides becoming more
noisy.
/:cl:
…ut still some) lag. (#73628)

Doesn't quite fix but helps mitigate #71697 - The issue report is still
valid, just less so.

Marked.js is just... Slow when used in BYOND's web browsing environment.
New markdown elements **and HTML tags** add significant fractions of a
millisecond to parsing time on my local.

However, I suspect I messed up with using the custom extensions and
every parsing pass it may have be appending a new extension to the
extension list.

I'm not sure why this never manifested in my 6 minutes of testing gif
from the original PR, mind you. I checked out that commit and local and
everything worked the same as I remember. Perhaps a change or version
bump in the interim modified the behaviour.

That aside, on current master it's MASSIVELY obvious something is
fucked.

Spamming 1000 characters of
```
```
then saving it, then puting **another** 1000 characters of the same
thing for 2000 previewed characters total gave the following performance
metrics:

![image](https://user-images.githubusercontent.com/24975989/221262868-c98d9b1a-e9d2-4a41-89e1-5ca6fb244cc6.png)

The marked.js parse time was about 150-180ms for the saved text and
220+ms for the input box text.

Moreso, if you leave the paper open for for a few minutes...

![image](https://user-images.githubusercontent.com/24975989/221263091-ae021caf-912d-4424-a33f-a91591ddf47e.png)

Yeah. That's 500+ms per parse (with 2 per render - one from the paper's
saved text and one from the input box) and the game will basically
crash.

Similar results using this template provided to me for testing purposes:
```
<center><b>Department Psychological Evaluation Survey</b></center>
<center><i>Test Log 230126-01</i></center>

<b>Question 1</b>
What department do you work in?
[_____________________________________]

<b>Question 2</b>
Which department are you least likely to recommend to a friend that they should work in?
<i>Please only choose one</i>
* Command [__]
* Security [__]
* Service [__]
* Cargo [__]
* Medical [__]
* Science [__]
* Engineering [__]

<b>Question 3</b>
Which department are you most likely to recommend to a friend that they should work in?
<i>Please only choose one</i>
* Command [__]
* Security [__]
* Service [__]
* Cargo [__]
* Medical [__]
* Science [__]
* Engineering [__]

<b>Question 4</b>
What is your favorite animal?
[_____________________________________]

---
Please sign here to confirm you are completing this survey voluntarily:
[_____________________________________]
<i>All Nanotrasen brand pens have an automatic signature system. Please write % s to automatically sign it.</i>
```

On first input + save, render time is normal but you can see it clearly
rising each second...

![image](https://user-images.githubusercontent.com/24975989/221263763-a0974f94-a95d-472a-881a-99ebf435239c.png)

![image](https://user-images.githubusercontent.com/24975989/221263878-5ed49878-aac2-49d1-9658-a7ff019e0947.png)

You get the idea.

Fixing the problem by using the proper markdown method for accomplishing
the same thing:

Markdown # Slow # Slower # Slowest test:

![image](https://user-images.githubusercontent.com/24975989/221264079-c58ec945-c130-4d3a-9dd7-b271b79321a3.png)

HoPaperwork Form

![image](https://user-images.githubusercontent.com/24975989/221264012-da121a5f-e4a8-4514-ab07-fb314ba50454.png)

And these numbers don't increase over time. So I think I nailed an
interim fix.

Ultimately, marked.js in tgui/BYOND just seems slow as molasses. But it
can at least be **usable** for now and this should majorly mitigate or
even eliminate problems players have been having with paper (depending
on how loaded it is with HTML and markdown tags).

All the features we had before seem to still work after I feex. So
hopefully good for now?

![image](https://user-images.githubusercontent.com/24975989/221267657-03fb0536-63a5-43a2-8f2b-b09bf7686d75.png)

I decided I'd go a step further and implement some basic caching logic.

![image](https://user-images.githubusercontent.com/24975989/221320748-54d60715-f8b8-48f7-9690-34467b526df2.png)

This means that you don't pay a parsing cost from already saved text
while writing new text, allowing complex forms to be created in multiple
saved or one big copy-paste.

This means that reading paper and filling in input boxes in paper is
free after the first parse, using the cache afterwards.

This is a workaround for the fact a parse can take longer than we'd like
for complicated paper forms.

Paper good.
:cl:
fix: Papercode has been significantly improved and trivially filled
paper forms should no longer lag or crash players' game clients.
/:cl:
## About The Pull Request
Fixes #71698

The paper copying proc was incomplete and didn't copy over the total
field count, which is used for sanity checking/user input verifying.

Because this wasn't copied over, none of the copied fields passed
validation checks for text input and the ui_act would have rejected the
inputs as invalid.
## Why It's Good For The Game
I feex.
## Changelog
:cl:
fix: Fixes broken input fields on photocopied paper.
/:cl:
…s (#75021)

## About The Pull Request

It turns out someone used regular length() in papers code which caused
all cyrillic and some other characters to count as two symbols even
though tgui itself shows and counts them as one. Should be working fine
now.

## Why It's Good For The Game

less bugs = better game

## Changelog
:cl:
fix: papers should count cyrillic and other non-ascii characters
correctly both server and client-side
/:cl:
@github-actions github-actions bot added Sound TGUI-Changes Contains changes to TGUI. Make sure its up to date with TGUI 4.0 labels Jul 15, 2023
@PowerfulBacon PowerfulBacon added this pull request to the merge queue Jul 21, 2023
Merged via the queue into BeeStation:master with commit 89a853b Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement Feature Fix Sound TGUI-Changes Contains changes to TGUI. Make sure its up to date with TGUI 4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants