refactors statpanel to use tgui API

[magatsuchi]

About The Pull Request

refactors the status panel to utilize the tgui/byond communication APIs instead of passing along href data, as well as converts the entirety of it into a datum/tgui_window

Why It's Good For The Game

communication between DM and html has never been pretty: utilizing location.href to provide a maximum of 2048 characters of string typed data. the stat panel serves as a suitable test subject for this relatively new API

[JohnFulpWillard]

https://github.com/tgstation/tgstation/blob/master/.github/guides/STANDARDS.md

Make sure it is still possible to use HTML

[magatsuchi]

https://github.com/tgstation/tgstation/blob/master/.github/guides/STANDARDS.md

Make sure it is still possible to use HTML

mothblocks concurs that unless MSO has a problem with it, utilizing datum/tgui_window in place of normal HTML writing (aka using the output operator) is fine

[magatsuchi]

(speaking of, @MrStonedOne you should look at this)

[stylemistake]

I'm turning it into a draft since it needs some polish.

TGUI rule does not apply here since this is a single page HTML with no assets.

[stylemistake]

Things I'd like you to address:

Instead of using one big Byond.subscribe and a bunch of separate functions, you should turn the callback function into this in-place construct, which makes it clear which functions are the actual functions, and which functions are callbacks to DM messages.

Byond.subscribeTo(type, function (payload) {
  // do not call an external function here
  // but instead handle all logic in place
});

e.g.

function update_sdql2(S) {
	sdql2 = JSON.parse(S);
	if(sdql2.length > 0 && !verb_tabs.includes("SDQL2")) {
		verb_tabs.push("SDQL2");
		addPermanentTab("SDQL2");
	}
	if(current_tab == "SDQL2")
		draw_sdql2();
}

This would be better as:

Byond.subscribeTo('update_sdql2', function (sdql2) {
	if(sdql2.length > 0 && !verb_tabs.includes("SDQL2")) {
		verb_tabs.push("SDQL2");
		addPermanentTab("SDQL2");
	}
	if(current_tab == "SDQL2")
		draw_sdql2();
});

Notice that you don't need to JSON.parse anything, nor do you need to manually json_encode it in DM, this is because objects that are sent via tgui message protocol are already json encoded, so you're basically json-encoding things twice, and there's no need to do that. What you send as payload is what you get in subscribe.

So you need to rework both subscribe functions and json_encode things such as this:

target.stat_panel.send_message("update_tickets", list(tickets = json_encode(ahelp_tickets)))

---

You need to move JS to a separate file because it's a lot of code, it messes up git diffs, it will be more convenient that way. Currently I see that inline_js doesn't work for you, I'll try to see what's the issue, ping me on Discord so that we could figure it out.

---

You should replace [addEventListenerKey] with .addEventListener and [textContentKey] with .textContent in all because polyfills for this API are included, this is the big reason why we're doing this.

---

window.onload = function () {
	setupSubscribe();
	NotifyByondOnload();
};

function NotifyByondOnload() {
	Byond.command("Panel-Ready")
}

Byond.subscribe() and subscribeTo() are not dependent on the load event, so they can subscribe right away, without being contained within onload callback or a wrapper function.

Panel-Ready verb is not doing anything useful at all:

/client/verb/panel_ready()
	set name = "Panel Ready"
	set hidden = TRUE

	statbrowser_ready = TRUE
	init_verbs()

Ready check is already implemented as part of tgui and you already used stat_panel.is_ready() in many places, so you need to only call Update-Verbs. And NotifyByondOnload wrapper function is unnecessary at all, just send the command directly in onload callback.

---

Many statpanel verbs are used as remote functions, they're pretty much all hidden, so instead you can use Byond.sendMessage and handle it in your choice of proc.

/client/New()
  ...
  stat_panel.subscribe(src, .proc/on_stat_panel_message)

/client/proc/on_stat_panel_message(type, payload)
  ...

smth like that.

[MrStonedOne]

https://github.com/tgstation/tgstation/blob/master/.github/guides/STANDARDS.md

Make sure it is still possible to use HTML

mothblocks concurs that unless MSO has a problem with it, utilizing datum/tgui_window in place of normal HTML writing (aka using the output operator) is fine

na, this actually isn't fine.

the chat box being tgui only makes it past my requirement because when tgui fails to load the chat box falls back to a byond output control.

because of how we did browser stat panels, this can not be done for them.

If tgui_panel didn't have the bluescreen stuff, or typescript, or any of the other cancerous "halt and break everything on 1 tiny error because thats what a good and proper:tm: modern:tm: language does" it could be workable for this usecase, but right now this is only gonna increase unreliability when before, on a minor script error, it would still try to keep chugging along.

[stylemistake]

If tgui_panel didn't have the bluescreen stuff

We can turn the bluescreen feature off for all panels as part of this PR, so non-issue. Good that you bring this up. 👍

tgui fails to load

This particular "tgui" doesn't have assets which is the primary reason why UIs can fail to load, so stat panel can't possibly "fail to load".

[MrStonedOne]

The primary reason uis fail to load is because they timeout loading because byond on some computers will randomly take 60 seconds to create the browser element while a white screen (from the empty window skin element with no contents) laughs at your face.

Luckily it seems the html panel system used by tgui chat doesn't have the annoying (but sadly necessary) timeout system, but given that the pieces this interfaces uses exist in mostly plain javascript in the interface, i do want to ensure that react failing to init for any reason doesn't inappropriately block the interface from working.

[stylemistake]

because byond on some computers will randomly take 60 seconds to create the browser element while a white screen (from the empty window skin element with no contents) laughs at your face.

That's your assumption, because general tgui windows will appear as white while they're fetching assets, and this means they might be actually running scripts already, you just don't see it. (I wanted to make them show a spinner at some point to distinguish it from just a white screen, but anyway).

I can guarantee with 99% probability it's caused by assets being slow or failing to download, which causes a retry (which is also invisible, until a certain point, then it fails with a blue screen).

i do want to ensure that react failing to init for any reason doesn't inappropriately block the interface from working.

Aye aye, cap'n!

[stylemistake]

^ doesn't help with loading unloadable icons, and causes new bugs

[san7890]

During Round 183219, I literally could not click any of the tabs to get the following changed (in order to see the contents of that tab).

I had died and was revived at this point, so maybe something in relation to having the tabs+verbs available to me? Sometimes as I moved around, the contents of that above screenshot would flash away and actually show me what I wanted to see (the "Status" Panel), but only for a second at most.

[magatsuchi]

During Round 183219, I literally could not click any of the tabs to get the following changed (in order to see the contents of that tab).

I had died and was revived at this point, so maybe something in relation to having the tabs+verbs available to me? Sometimes as I moved around, the contents of that above screenshot would flash away and actually show me what I wanted to see (the "Status" Panel), but only for a second at most.

did the tgui log show anything?