Vue.js UI implementation #4868

Karolis2011 · 2018-06-06T20:44:46Z

This UI is going to be more integrated with BYOND host objects. It's update principal is very different from nanoui's. It is based around state that is being synchronized with server and client (browser). Such synchronization has it's problems, like it can't handle rapid changes, what could cause client and server to become out of sync and client state to be discard.
Progress so fair:

Extras that come with this PR:

Travis now has stages for things

I am open to comments and criticism regarding this framework.

Lohikar · 2018-06-06T20:51:47Z

code/modules/byvue/external.dm

+/datum/proc/byvue_state_change(var/list/newstate, var/mob/user, var/datum/byvueui/ui)
+    return
+
+/mob/var/list/open_byvue_uis = list()


Eventually this should be made lazy - = list() vars on core types are bad.

Lohikar · 2018-06-06T20:52:32Z

code/controllers/subsystems/processing/byvue.dm

@@ -0,0 +1,79 @@
+var/datum/controller/subsystem/processing/byvue/SSbyvue


Might be a good idea to make it clearer that this is a UI SS?

Lohikar · 2018-06-06T20:53:21Z

code/modules/byvue/ui.dm

+    // object that contains this ui
+    var/datum/object
+    // apperant object that contains ui, used for checks
+    var/atom/wobject = null


What's the difference between object and wobject?

object is the one receiving calls, and managing data, wobject is optional supplemented object that is present in world and it's coordinates can be checked like, if it is near user. if object is already an atom, then it should be used for checks, if it is not, then wobject. if both are unavailable for checks, then it should be used as 'admin' ui.

skull132 · 2018-06-08T23:18:39Z

code/controllers/subsystems/processing/vueui.dm

+        return 0	// Wasn't open.
+
+    STOP_PROCESSING(SSvueui, ui)
+    if(ui.user)	// Sanity check in case a user has been deleted (say a blown up borg watching the alarm interface)


if (QDELETED(ui.user))

no?

if(!QDELETED(ui.user))

yes?

Karolis2011 · 2018-06-18T21:50:34Z

Implemented first ui in this framework, looks good so fair.

Karolis2011 · 2018-06-22T20:37:46Z

Karolis2011 · 2018-06-27T20:12:02Z

Well, this should fix #4714 as it replaces fax machine ui and adds checks,

Karolis2011 · 2018-07-01T09:43:09Z

Also old icon set copied from nanoui was swaped out for material iconset.
For reference, as of next commit available themes will be and look like:
Nano Dark:

Nano Light:

Basic Light:

Basic dark:

skull132 · 2018-07-01T19:52:11Z

.travis.yml

-  - (! grep -E "<\s*span\s+class\s*=\s*('[^'>]+|[^'>]+')\s*>" **/*.dm)
-  - (num=`grep -E '\\\\(red|blue|green|black|b|i[^mc])' **/*.dm | wc -l`; echo "$num escapes (expecting ${MACRO_COUNT} or less)"; [ $num -le ${MACRO_COUNT} ])
-  - awk -f tools/indentation.awk **/*.dm
-  - md5sum -c - <<< "94c0d540b3b0f008fbc4353e667de690 *html/changelogs/example.yml"
  - python tools/TagMatcher/tag-matcher.py ../..


This is a code check. So is the step below it as well, they should both be there.

skull132 · 2018-07-01T19:52:32Z

SQL/migrate/V029__HTML_UI_preferences.sql

@@ -0,0 +1,6 @@
+--


Migration needs a version bump.

skull132 · 2018-07-01T19:56:19Z

code/controllers/subsystems/processing/vueui.dm

+  */ 
+/datum/controller/subsystem/processing/vueui/proc/get_open_uis(var/src_object)
+    var/src_object_key = SOFTREF(src_object)
+    if (!LAZYLEN(open_uis[src_object_key]))


This proc can be done without the if-check. Accessing a key that is not set in the list, as long as the key is not a number, returns null anyways. So:

/datum/controller/subsystem/processing/vueui/proc/get_open_uis(var/src_object) var/src_obj_key = SOFTREF(src_object) . = open_uis[src_object_key]

skull132 · 2018-07-01T19:56:37Z

code/controllers/subsystems/processing/vueui.dm

+  *
+  * @param src_object - object that hosts the ui we are looking for
+  *
+  * @return list of ui datums


this comment is a lie.

List of UI datums or null.

skull132 · 2018-07-01T19:56:57Z

code/controllers/subsystems/processing/vueui.dm

+  * @param user - user that has that ui open
+  * @param src_object - object that hosts the ui we are looking for
+  *
+  * @return ui datum


A lie.

UI datum or null.

skull132 · 2018-07-01T19:57:12Z

code/controllers/subsystems/processing/vueui.dm

@@ -0,0 +1,173 @@
+var/datum/controller/subsystem/processing/vueui/SSvueui


This entire file is indented with spaces. Please reindent with tabs.

skull132 · 2018-07-01T19:57:38Z

code/controllers/subsystems/processing/vueui.dm

+  *
+  * @param src_object - object that hosts ui that should be updated
+  *
+  * @return nothing


Can be discarded.

skull132 · 2018-07-01T20:03:37Z

code/modules/vueui/external.dm

@@ -0,0 +1,23 @@
+/client/verb/vueuiclose(var/uiref as text)
+    set hidden = 1	// hide this verb from the user's panel


Space indented file.

skull132 · 2018-07-01T20:05:29Z

code/modules/vueui/ui.dm

@@ -0,0 +1,321 @@
+/*


Space indented file.

Lohikar · 2018-07-01T19:56:23Z

code/controllers/subsystems/processing/vueui.dm

@@ -0,0 +1,173 @@
+var/datum/controller/subsystem/processing/vueui/SSvueui
+
+#define NULL_OR_EQUAL(self,other) (!(self) || (self) == (other))


This should probably be just moved to misc defines if it's used in more than one place (NUI).

Lohikar · 2018-07-01T19:58:10Z

code/modules/paperwork/photocopier.dm

@@ -1,3 +1,4 @@
+#define VUEUI_SET_CHECK(a, b, c, d) if (a != b) { a = b; c = d; }


If this is used in multiple places, it should probably go in misc defines.

Lohikar · 2018-07-01T19:59:02Z

code/modules/vueui/themes.dm

+#define THEME_TYPE_DARK 1
+#define THEME_TYPE_LIGHT 0
+
+/var/available_html_themes = list(


Maybe this should be a var on SSvueui?

This also should be typed as /list.

Lohikar · 2018-07-01T19:59:36Z

code/modules/vueui/ui.dm

+Byond Vue UI framework
+main ui datum.
+*/
+#define UIDEBUG 0


Why not just use #ifdef gating instead of checking a boolean?

Lohikar · 2018-07-01T20:00:14Z

code/modules/vueui/ui.dm

+    cl << browse_rsc(file("vueui/dist/main.js"), "vueui.js")
+    cl << browse_rsc(file("vueui/dist/main.css"), "vueui.css")
+#else
+    var/datum/asset/assets = get_asset_datum(/datum/asset/simple/vueui)


I think there's a way to check if an asset has already been sent to the client instead of re-sending it each time?

Lohikar · 2018-07-01T20:05:34Z

vueui/README.md

+### `ui.header`
+Determines header component that is used with this ui. Should be set before `ui.open()`.
+### `ui.auto_update_content`
+Constantly checks for change using `ui.check_for_change()`. This should be only used when data changes unpredictably , 


Trailing ,. What do you mean by constantly?

Lohikar · 2018-07-01T20:06:10Z

vueui/README.md

+### `ui.add_asset(name, image)`
+Adds asset for ui use, but does not send it to client. It will be sent in during next `ui.open()` call or if it's done manually with `ui.send_asset(name)` combined with `push_change(null)`.
+### `ui.remove_asset(name)`
+Removes asset from future use in ui. But client-side asset index isn't updated immediately to reflect removal of asset.


I don't believe the asset cache supports removing assets. It's also global.

This asset part isn't part of asset cache, it's special server generated image asset management proc. Was designed mostly having records in mind.

Lohikar · 2018-07-01T20:06:54Z

vueui/README.md

+### `ui.push_change(data).`
+Pushes data change to client. This also pushes changes to metadata, what includes: title, world time, ui status, active ui component, client-side asset index.
+### `ui.check_for_change(forcedPush)`
+Checks with `object.vueui_data_change` if data has changed, if so, then change is pushed. If forcedPush is resolving to true, then it pushes change anyways.


"is resolving to true" is a bit awkward; maybe "If forcedPush is true"?

Lohikar · 2018-07-01T20:07:15Z

vueui/README.md

+### `ui.check_for_change(forcedPush)`
+Checks with `object.vueui_data_change` if data has changed, if so, then change is pushed. If forcedPush is resolving to true, then it pushes change anyways.
+### `ui.update_status()`
+This call should be used if external change was detected. It checks if user still can use this ui, and what's it usability level.


it => its

Lohikar · 2018-07-01T20:09:49Z

vueui/README.md

+Topic status, used to determine how interactive is ui. Meanings of numbers can be seen `\code\__defines\machinery.dm`.
+#### `assets` - This makes `add_asset`, `remove_asset` and `<vui-img>` tick
+As described, this makes them tick. This ***should*** shouldn't be used, unless you know what you are doing.
+#### `active` - Representation of `ui.activeui`


What exactly does this var do?

It mostly tells ui what template should be used.

Sindorman

Right, I do not know JS, nor I am familiar with Vue. To me JS looks like some merge of Java and python. Anyways, I know JSON and it seems that all code involving JSON read/write encoding/decoding is correct and JSON files are to the format. I would say that this is good!

I can't say if it is easy to use or not!

Arrow768

Looks good.
I´ll try to build something with it in the next days to see if there are any issues / questions that should be answered in the readme

Alberyk

Good so far, it does not seem more complex than what we already use, and I had a positive experience when we did a run test with this implementation.

skull132

Overall, I looked over the mechanics of this and the actual VueUI implementation of this. And it looks relatively good on that end as well. The ability to utilize component variables and the like is neat. So this seems great on that count.

~~It would actually be super if we could utilize each template's own variables, instead of having a state global, but I'm unsure how effective and realistic this would be to implement.~~ So this works and works well enough.

Also, minor code things.

skull132 · 2018-07-29T19:02:23Z

code/game/machinery/atmo_control.dm

@@ -93,16 +93,27 @@ obj/machinery/computer/general_air_control/Destroy()
 		SSradio.remove_object(src, frequency)
 	return ..()

+/obj/machinery/computer/general_air_control/vueui_data_change(var/list/data, var/mob/user, var/vueui/ui)
+	if(!data)
+		. = data = list("sensors" = list())


This should be . = data || list("sensors" = list()) I'm psure?

I am pretty sure that doesn't set data var, this is mostly short hand for, and having same list ref

. = list("sensors" = list()) data = list("sensors" = list())

skull132 · 2018-07-29T19:03:42Z

code/game/machinery/atmo_control.dm

-
-	output += "<B>Tank Control System</B><BR><BR>"
+/obj/machinery/computer/general_air_control/large_tank_control/vueui_data_change(var/list/data, var/mob/user, var/vueui/ui)
+	. = ..()


This can be shortened to: data = ..() || data I thiiink? If ..() returns null, it'll swap back to data.

But if ..() returns not null, . will be unset and no way to know if it returned not null, to be able to set . at a latter point.

skull132 · 2018-07-29T19:04:27Z

code/game/machinery/atmo_control.dm

-
-	output += "<B>Core Cooling Control System</B><BR><BR>"
+/obj/machinery/computer/general_air_control/supermatter_core/vueui_data_change(var/list/data, var/mob/user, var/vueui/ui)
+	. = ..()


Same here, data = ..() || data

Although you'll also want to set return value I think?

skull132 · 2018-07-29T19:05:28Z

code/game/machinery/atmo_control.dm

@@ -341,62 +271,44 @@ Min Core Pressure: [pressure_limit] kPa<BR>"}

 /obj/machinery/computer/general_air_control/supermatter_core/Topic(href, href_list)


The return value should not be removed from this proc! It's meaningful.

How will it be used? VueUI doesn't use Topic() return value to do anything, it's just droped.
Look at https://github.com/Aurorastation/Aurora.3/pull/4868/files/4babde452dbc70d167be6f302f2e0110784cd917#diff-384c1ecb0c07ace2ea7785c878f6e836R244

skull132 · 2018-07-29T19:05:41Z

code/game/machinery/atmo_control.dm

 		output_info = null
-		signal.data = list ("tag" = output_tag, "set_external_pressure" = "[pressure_setting]", "checks" = 1)
-		. = 1
+		signal.data = list ("tag" = output_tag, "set_external_pressure" = "[setpressure]", "checks" = 1)

 	signal.data["sigtype"]="command"
 	radio_connection.post_signal(src, signal, filter = RADIO_ATMOSIA)

 	spawn(5)


Timer this.

skull132 · 2018-07-29T19:06:17Z

code/game/machinery/atmo_control.dm

-
-	output += "<B>Fuel Injection System</B><BR>"
+/obj/machinery/computer/general_air_control/fuel_injection/vueui_data_change(var/list/data, var/mob/user, var/vueui/ui)
+	. = ..()


data = ..() || data

skull132 · 2018-07-29T19:10:46Z

code/modules/paperwork/photocopier.dm

+	watch_var("copyitem", "gotitem", CALLBACK(null, .proc/transform_to_boolean, FALSE))
+
+/obj/machinery/photocopier/vueui_data_change(var/list/data, var/mob/user, var/vueui/ui)
+	var/monitordata = ..()


Psure you could make this proc use an early return somehow? Though I can't figure it out atm. 🤔

How about this?

. = ..() if (.) data = .

skull132 · 2018-07-29T19:12:41Z

code/modules/vueui/ui.dm

+	send_resources_and_assets(user.client)
+	user << browse(generate_html(), params)
+	winset(user, "mapwindow.map", "focus=true")
+	spawn(1)


Timer callback time.

YaBoiElliot

Wow. To start off, a lot of good work has clearly gone into this. That said I know next to nothing about Vue.js other than its' existance and why it is being used here.

Giving a brief look over of this PR it looks very very solid currently and I can not fault it. I wish I could provide more but I can't find anything faulty or anything that needs improving that hasn't already been mentioned.

For whatever it's worth, this is looking excellent and, assuming it's given a thorough test, shouldn't be an issue with adding.

…e.Added neat thing when you open template.html

PJB3005 · 2018-10-10T20:25:16Z

Oh jeez how come this one took months before I knew of it.

Is this another "people who have no idea how Nano worked re-inventing Nano and failing" episode?

I looked at the docs in the repo and they look incredibly unwieldy to work with.

Also Nano was absolutely perfect. Change my mind.

This PR is depending on #4868 for it's ui framework. This PR mostly makes new SSrecords subsystem responsible for storing records. This should replace old datacore. Make new SSrecords. Make things use SSrecords and whole code compile Made VueUi button <vui-button> to push parameters as JSON, preserving client side data stricture. Add new records console and admin record management. I am mostly looking for feedback regarding SSrecords and it's data storage mechanism criticism (It's using lists for storage)

…rastation#4878) This PR is depending on Aurorastation#4868 for it's ui framework. This PR mostly makes new SSrecords subsystem responsible for storing records. This should replace old datacore. Make new SSrecords. Make things use SSrecords and whole code compile Made VueUi button <vui-button> to push parameters as JSON, preserving client side data stricture. Add new records console and admin record management. I am mostly looking for feedback regarding SSrecords and it's data storage mechanism criticism (It's using lists for storage)

Lohikar reviewed Jun 6, 2018

View reviewed changes

Karolis2011 requested a review from Arrow768 as a code owner June 8, 2018 18:59

skull132 reviewed Jun 8, 2018

View reviewed changes

Karolis2011 changed the title ~~[WIP] Vue.js UI implementation~~ Vue.js UI implementation Jun 11, 2018

Karolis2011 mentioned this pull request Jun 11, 2018

New record consoles and removal of old datacore, adds SSrecords #4878

Merged

4 tasks

skull132 added the WIP The PR is a work in progress and should not be reviewed yet. label Jun 16, 2018

skull132 suggested changes Jul 1, 2018

View reviewed changes

Lohikar reviewed Jul 1, 2018

View reviewed changes

BotBOREALIS added Changelog Required Review Required 🗺️ Mapping - Aurora The PR touches the Aurora map files. Mapping: Exodus and removed Changelog Required WIP The PR is a work in progress and should not be reviewed yet. labels Jul 1, 2018

Karolis2011 added 12 commits July 5, 2018 23:03

Start of vue

513331f

I got no idea what exactly am I doing

d53cc66

Some updates

94c4830

Note: Edge is a bad browser

e38ee58

Something is working

fdaa4b8

Made things behave

18387b5

It works kindda somehow

41d7e02

Things had to change

138d26b

Ugh... I did messup this

47916ff

Added state and un nano-ified one thing

615c02e

Some messups, again

b519796

Documentation and few upgrades

7489492

Karolis2011 added 2 commits July 28, 2018 20:39

Adds code owners

d5c1cd0

Merge remote-tracking branch 'upstream/development' into byvue

4babde4

Sindorman approved these changes Jul 29, 2018

View reviewed changes

Arrow768 reviewed Jul 29, 2018

View reviewed changes

Alberyk reviewed Jul 29, 2018

View reviewed changes

skull132 suggested changes Jul 29, 2018

View reviewed changes

Karolis2011 added 3 commits July 29, 2018 23:12

USe of timers

17b60a6

More compactness

357c22a

More compact atmo control

0ce343e

YaBoiElliot reviewed Jul 30, 2018

View reviewed changes

Karolis2011 added 9 commits August 2, 2018 16:25

Updates, improvements and fixes

501d1f7

Cleanupname field on components was pointless and was confusing peapl…

83793d7

…e.Added neat thing when you open template.html

Fun of travis begins once again.

c9c34df

I forgot to handle this

8381602

Betetr build process?

b5ac84a

Let's hope things workout

59e47c1

Grammar and spell check

3958d4a

Merge branch 'development' into byvue

bdac595

Updates .gitignore to try to respect new policies

17c59cf

BotBOREALIS added the 🚫 Do Not Merge label Aug 2, 2018

skull132 added this to the August Update milestone Aug 4, 2018

skull132 approved these changes Aug 4, 2018

View reviewed changes

Merge branch 'development' into byvue

6a9785c

skull132 merged commit 4065e29 into Aurorastation:development Aug 4, 2018

skull132 added a commit to skull132/Aurora.3 that referenced this pull request Aug 5, 2018

Fixes travis error introduced in Aurorastation#4868

0a22662

skull132 added a commit that referenced this pull request Aug 5, 2018

Fixes travis error introduced in #4868 (#5082)

84d7d34

		@@ -0,0 +1,79 @@
		var/datum/controller/subsystem/processing/byvue/SSbyvue

		@@ -0,0 +1,173 @@
		var/datum/controller/subsystem/processing/vueui/SSvueui

		@@ -0,0 +1,23 @@
		/client/verb/vueuiclose(var/uiref as text)
		set hidden = 1 // hide this verb from the user's panel

		@@ -0,0 +1,173 @@
		var/datum/controller/subsystem/processing/vueui/SSvueui

		#define NULL_OR_EQUAL(self,other) (!(self) \|\| (self) == (other))

		@@ -1,3 +1,4 @@
		#define VUEUI_SET_CHECK(a, b, c, d) if (a != b) { a = b; c = d; }

		@@ -341,62 +271,44 @@ Min Core Pressure: [pressure_limit] kPa<BR>"}

		/obj/machinery/computer/general_air_control/supermatter_core/Topic(href, href_list)

Vue.js UI implementation #4868

Vue.js UI implementation #4868

Conversation

Karolis2011 commented Jun 6, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Karolis2011 commented Jun 18, 2018

Karolis2011 commented Jun 22, 2018

Karolis2011 commented Jun 27, 2018

Karolis2011 commented Jul 1, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sindorman left a comment

Choose a reason for hiding this comment

Arrow768 left a comment

Choose a reason for hiding this comment

Alberyk left a comment

Choose a reason for hiding this comment

skull132 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YaBoiElliot left a comment

Choose a reason for hiding this comment

PJB3005 commented Oct 10, 2018

Karolis2011 commented Jun 6, 2018 •

edited

Karolis2011 commented Jul 1, 2018 •

edited