Custom task logic via extending InteractTask #300

TheSecondReal0 · 2021-02-13T03:42:26Z

Added a bunch of virtual functions meant to be overridden to interacttask.gd
Added functions in both interacttask.gd and the task manager to facilitate syncing task resource scripts (they are basically proxies for rpc and rset)
Ported the clockset task connected to the standbutton to use a script extending InteractTask for logic (tasks are still completed via the UI class)
Task networking supports RPC modes
Task resources now tell the task manager when they are complete

I'll type up some documentation at some point, to see an example of using the InteractTask resource for logic look at the clockset task (resource script is at res://assets/ui/tasks/clockset/clocksetinteracttask.gd)

TODO:

~~Have task resources tell the task manager when they are completed~~
~~Add some security to task resource rpc and rset, currently any peer can do whatever they want~~
~~Maybe change task manager to use task IDs instead of task_info dicts, @Damjan94 what is your opinion on this?~~ Necessary to make sure you know which player(s) the call is referring to

NOTES:

You must make your InteractTask extending script a tool script, otherwise the custom editor properties won't work

github-actions · 2021-02-13T03:42:39Z

HOW TO REVIEW:

Test multiplayer (client and host)
Check build status
Check UI
Check the code
Check build artifacts

nicemicro

Quite cool!

nicemicro · 2021-02-13T04:29:55Z

src/addons/opensusinteraction/resources/interacttask/interacttask.gd

+func task_rpc(function: String, args: Array):
+	TaskManager.task_rpc(function, args, task_id)
+
+func task_rpc_id(id: int, function: String, args: Array):
+	TaskManager.task_rpc_id(id, function, args, task_id)
+
+func receive_task_rpc(function: String, args: Array):
+	# using callv instead of call because it will translate the array into
+	# 	individual arguments
+	callv(function, args)


couldn't it be done so that task resources can only RPC the server, and can only accept RPCs from the server?

Yeah, it could be done that way. I set it up like this so each task can fully customize their networking stuff 😃

functions with virtualized counterparts moved to the top, networking functions are below those, then setters/getters, then code for custom editor properties

overhauled how task manager handles completing tasks and somewhat how it handles sync

to make sure you can't go through UI to complete a task

This reverts commit fca6bda.

nicemicro

Besides the questions with respect to the code, the following bugs need to be addressed:

The task list fails to color the completed tasks green.
When the clockset task (standbutton) is completed, the text showing the time next to the standbutton is only changed on the other players' map if they stand on the button.
On the interact button version, the text is only updated if the other player goes there and clicks on the task test button.

I'd suggest that 2. and 3. should be addressed that every time there is an update broadcasted to all clients, the script should update the text.

nicemicro · 2021-02-19T12:58:49Z

src/assets/ui/tasks/clockset/clockset.gd

+	return get_res().get_current_time()
+
+func get_res() -> Resource:
+	return TaskManager.get_task_resource(ui_data[TaskManager.TASK_ID_KEY])


I am trying to figure out where is the ui_data dictionary gets populated with the correct task ID, but I can't figure it out. Please enlighten me.

It is updated by uicontroller.gd right before it's opened

nicemicro · 2021-02-19T13:14:30Z

src/assets/ui/tasks/clockset/clocksetinteracttask.tres

+[gd_resource type="Resource" load_steps=4 format=2]
+
+[ext_resource path="res://assets/ui/tasks/clockset/clocksetinteracttask.gd" type="Script" id=1]
+[ext_resource path="res://addons/opensusinteraction/resources/interactui/interactui.gd" type="Script" id=2]
+
+[sub_resource type="Resource" id=1]
+resource_local_to_scene = true
+resource_name = "InteractUI"
+script = ExtResource( 2 )
+ui_name = "clockset"
+ui_data = {
+
+}
+action = 0
+advanced/reinstance = false
+advanced/free_on_close = false
+
+[resource]
+resource_local_to_scene = true
+script = ExtResource( 1 )
+task_text = "clockset task"
+ui_resource = SubResource( 1 )
+outputs/toggle_map_interactions = true
+outputs/output_map_interactions = [  ]
+is_task_global = true


What is the purpose of this file?
I saw that the map features use the interactpointclockset.tres and standbuttonclockeset.tres resources as Interact Resource in their respective points, however those resources are linked to clocksetinteracttask.gd as their script. However, this tres file seems to be referenced nowhere.

It was mostly for testing, and so I could make a copy of it to make the resource files used in the test map. Just putting that resource into the map either did really weird stuff (like adding a copy of the script inside test.tres) or I couldn't give them unique task text (despite them both being local to scene).

TheSecondReal0 · 2021-02-19T17:00:32Z

@nicemicro I think bugs 2 and 3 are caused by incorrect syncing of the clockset task, which would explain why it works perfectly if you complete the task on the host. I'll fix the syncing issues, and I'll make sure the task state is actually transitioned as @Damjan94 brought up in the Matrix chat

* Task color is now updated in UI * Task is now remotely completed more reliably

TheSecondReal0 · 2021-02-19T18:06:24Z

Bugs 2 and 3 were actually caused by attempt_complete_task() being called normally instead of via RPC

Damjan94

If you make any task not global

and try to complete it, the game crashes...

nicemicro

I have found the following issues, please take a look.

Non-global tasks have the same final state (clock hands) and the states ("digital" clock) are synced through clients
Attempting to finish a non-global task on a client when the server is ran from the editor, results in an assertion error on the server in taskinfo.gd _on_task_completed() function, line 30
Attempting to finish a non-global task on a client when the client is ran from the editor doesn't cause any issues.
Attempting to finish a non-global task on a client not ran from the editor causes an error on a client that's ran from the editor in interacttask.gd transition() function, line 241 "Invalid get index (on base: Dictionary).
When all clients and the server are ran not from the editor, issues 2, 3 and 4 are not reproduced, however issue 1 causes the behavior that when someone finishes their clockset task, the second person who has the same personal task finishes the task immediately on opening it due to the state syncing.
If two players are attending to the same global task, and one changes the state, closes the window, the second player's display changes which is expected behavior. However, if one of the players is killed while their task window is open, they can still change the task state and when closing the window it gets synced across the network.

src/addons/opensusinteraction/resources/interacttask/interacttask.gd

TheSecondReal0 · 2021-02-21T05:54:41Z

To respond to your review @nicemicro

This is because I wrote the clockset logic with only global tasks in mind, I'll alter it accordingly
That assertion should be removed, as it will trigger an error when the game is functioning correctly
This seems to just be a note
This might be caused by some weird syncing, I'll take a closer look in the future
This is also because the clockset logic is only written to be a global task, like 1
This should probably fall under another PR, since this deals with death behavior (there should probably be a signal from PlayerManager which the UI system would use to close UIs). There could also be a default check added in sync_task() for whether or not the main player is alive

* Removed default inputs to some functions to ensure compatibility with personal tasks * Removed assert in taskinfo.gd which would fire incorrectly * Added a check to make sure tasks aren't completed twice * Improved personal task implementation of clockset task

New commits address the previous issues

nicemicro

I found the following issue:

The global clockset task have different different goal times on each clients.
When the first player opens the task window and closes it, the goal time gets set to the goal of the first player to everyone else.
The text next to the stand button shows the same time for everyone, but it only matches with the time on the client that was the last to join the party.
The text next to the stand button is only updated in case of successful completion of the task. Is it possible to update the related map item the same way when the task UI is updated, i.e. every time someone closes the task UI / changes some important parameter, so there could be some indication of the state of a task directly on the map in some cases?

TheSecondReal0 · 2021-02-24T19:53:00Z

Addressed issues 1, 2, and I think 3 by adding player data syncing (sync occurs when the server sends task assignments)
To address issue 4, I made it so that whenever times are synced on global clockset tasks it will update the map text, and personal tasks will update map text if it's assigned to you

nicemicro

This! Is! Approved!

Changes have been already implemented.

TheSecondReal0 added 9 commits February 11, 2021 23:59

groundwork for networking + extending InteractTask

2ad950e

Merge branch 'task-resource-despaghetti' into custom-task-logic

094eb3b

more work to allow custom interacttask scripts

12e0703

more virtual functions + comments

8ba4b5a

comment improvement

343cde8

added transition logic and more comments

b48705f

updated task manager to work with resource transition logic

b423739

began porting clockset task

ea0a2ee

finished porting clockset task

ea27f56

nicemicro requested review from nicemicro and Damjan94 February 13, 2021 04:03

nicemicro reviewed Feb 13, 2021

View reviewed changes

TheSecondReal0 added 2 commits February 16, 2021 12:40

improved networking

a6f6b07

reorganized interacttask.gd

7384d2e

functions with virtualized counterparts moved to the top, networking functions are below those, then setters/getters, then code for custom editor properties

This was linked to issues Feb 17, 2021

Tasks system #196

Closed

Maintenance tasks #198

Closed

TheSecondReal0 added 4 commits February 18, 2021 20:38

TaskManager overhaul

598a1d9

overhauled how task manager handles completing tasks and somewhat how it handles sync

updated clockset task and fleshed out some stuff

a3957f3

cleaned up a bit

385d531

removed complete_task() from UI classes

4b3c8da

to make sure you can't go through UI to complete a task

TheSecondReal0 marked this pull request as ready for review February 19, 2021 02:48

TheSecondReal0 added 5 commits February 18, 2021 21:52

removed random numbers from InteractTask

fca6bda

Revert "removed random numbers from InteractTask"

ee00d85

This reverts commit fca6bda.

removed random numbers from TaskInteract without updating resources

9942ddc

updated resources to reflect random numbers removal

2be12bd

got resources figured out

3d0b83d

TheSecondReal0 requested a review from nicemicro February 19, 2021 03:29

added some comments

8f40ff2

nicemicro requested changes Feb 19, 2021

View reviewed changes

TheSecondReal0 added 2 commits February 19, 2021 12:37

fixed bugs

8cbcf94

* Task color is now updated in UI * Task is now remotely completed more reliably

actually fixed the desync bug

a1e1dfb

nicemicro previously approved these changes Feb 20, 2021

View reviewed changes

Damjan94 previously requested changes Feb 20, 2021

View reviewed changes

fixed crash that occurred with non-global tasks

922036f

TheSecondReal0 dismissed nicemicro’s stale review via 922036f February 20, 2021 23:21

fixed weird bug where task would be completed over and over again

bee22dd

TheSecondReal0 requested review from Damjan94 and nicemicro February 20, 2021 23:31

nicemicro previously requested changes Feb 21, 2021

View reviewed changes

src/addons/opensusinteraction/resources/interacttask/interacttask.gd Show resolved Hide resolved

TheSecondReal0 added 2 commits February 22, 2021 17:50

working on personal task support

2a508dc

general polishing

ffe2c9d

* Removed default inputs to some functions to ensure compatibility with personal tasks * Removed assert in taskinfo.gd which would fire incorrectly * Added a check to make sure tasks aren't completed twice * Improved personal task implementation of clockset task

TheSecondReal0 requested a review from nicemicro February 22, 2021 23:20

TheSecondReal0 added 2 commits February 22, 2021 23:29

another check to prevent trying to complete a task twice

66c2605

delete binary

9084397

nicemicro requested changes Feb 23, 2021

View reviewed changes

TheSecondReal0 added 2 commits February 24, 2021 14:26

improved task syncing

5eb802c

clockset map text updates more often

d79c7d8

TheSecondReal0 requested a review from nicemicro February 24, 2021 19:48

possible clockset times now include all intended values

161f71c

nicemicro approved these changes Feb 25, 2021

View reviewed changes

nicemicro merged commit 1d343a5 into opensuspect:main Feb 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom task logic via extending InteractTask #300

Custom task logic via extending InteractTask #300

TheSecondReal0 commented Feb 13, 2021 •

edited

Loading

github-actions bot commented Feb 13, 2021

nicemicro left a comment

nicemicro Feb 13, 2021

TheSecondReal0 Feb 13, 2021

nicemicro left a comment •

edited

Loading

nicemicro Feb 19, 2021

TheSecondReal0 Feb 19, 2021

nicemicro Feb 19, 2021

TheSecondReal0 Feb 19, 2021

TheSecondReal0 commented Feb 19, 2021 •

edited

Loading

TheSecondReal0 commented Feb 19, 2021

Damjan94 left a comment

nicemicro left a comment

TheSecondReal0 commented Feb 21, 2021

nicemicro left a comment

TheSecondReal0 commented Feb 24, 2021

nicemicro left a comment

Custom task logic via extending InteractTask #300

Custom task logic via extending InteractTask #300

Conversation

TheSecondReal0 commented Feb 13, 2021 • edited Loading

github-actions bot commented Feb 13, 2021

nicemicro left a comment

Choose a reason for hiding this comment

nicemicro Feb 13, 2021

Choose a reason for hiding this comment

TheSecondReal0 Feb 13, 2021

Choose a reason for hiding this comment

nicemicro left a comment • edited Loading

Choose a reason for hiding this comment

nicemicro Feb 19, 2021

Choose a reason for hiding this comment

TheSecondReal0 Feb 19, 2021

Choose a reason for hiding this comment

nicemicro Feb 19, 2021

Choose a reason for hiding this comment

TheSecondReal0 Feb 19, 2021

Choose a reason for hiding this comment

TheSecondReal0 commented Feb 19, 2021 • edited Loading

TheSecondReal0 commented Feb 19, 2021

Damjan94 left a comment

Choose a reason for hiding this comment

nicemicro left a comment

Choose a reason for hiding this comment

TheSecondReal0 commented Feb 21, 2021

nicemicro left a comment

Choose a reason for hiding this comment

TheSecondReal0 commented Feb 24, 2021

nicemicro left a comment

Choose a reason for hiding this comment

TheSecondReal0 commented Feb 13, 2021 •

edited

Loading

nicemicro left a comment •

edited

Loading

TheSecondReal0 commented Feb 19, 2021 •

edited

Loading