Container tasks #293

zapzoop0099 · 2021-01-30T13:52:55Z

A container task as intended by the design document
Currently it is fully functional/completed From Game Perspective but enough for getting items from the box

This PR provides:-

A full sync between the task
A location from where player can pick up the items

…o container-tasks

github-actions · 2021-01-30T13:53:14Z

HOW TO REVIEW:

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

Damjan94

This is the first change request. I just glanced over the code, and made suggestions.
I also checked out the code, and it seems that if I take all of the items out of the container in one go(ie, I only open the window once), the items end up being on the ground, and when I try to pick them up, they teleport to the centare(0,0) of the map...
I will make another review if I find more bugs.

Damjan94 · 2021-01-30T18:33:45Z

src/assets/autoload/uimanager.gd

@@ -85,6 +87,11 @@ func free_ui(ui_name: String):
 		push_error("free_ui() called with invalid ui name " + ui_name)
 	emit_signal("free_ui", ui_name)

+func pre_ins(ui_name:String):


what does this function do(maybe a more descriptive name)?

Damjan94 · 2021-01-30T21:01:49Z

src/assets/main/maps.gd

@@ -68,6 +68,8 @@ func switchMap(newMap: String) -> void:
 	# actual current map to position 0 manually.
 	move_child(mapClone, 0)
 	emit_signal("spawn", getSpawnPoints())
+	if newMap == "Test":
+		init_all_tasks()


This could be moved below line 128 in uimnager, because it is ui based, and that's where I have put open_ui("rolescreen").

wait for other peoples input, before you change this one, as I'm not 100% sure myself

Damjan94 · 2021-01-30T21:08:28Z

src/assets/maps/test/Container.gd

+
+func register(body) -> void:#Register a body
+	interactor[body.id] = body.get_path()
+	interactor["bool"] = true


What does this bool do? more descriptive names can never hurt

Damjan94 · 2021-01-30T21:22:36Z

src/assets/maps/test/Container.gd

+			if interactor.empty():
+				register(bodies)
+			else:
+				return


this can be better structured as

# Can't register a new body, if there is an already registered body if is_body_registered(): return for bodies in get_overlapping_bodies(): if not bodies.is_in_group("players"): continue register(bodies) return func is_body_registered() -> bool: return interactor.size() > 0

Damjan94 · 2021-01-30T21:29:26Z

src/assets/ui/tasks/container/container.gd

+	for slot in grid.get_children():
+		slot.ui_data.clear()
+
+puppetsync func erase_child():#erases all child


erase_children, plural of child is children

Damjan94 · 2021-01-30T21:32:15Z

src/assets/ui/tasks/container/container.gd

+	if not get_tree().is_network_server():
+		return
+	rpc("reset")
+remotesync func erase_child_server():


don't forget to change it to children here, too... English is sometimes complicated like that(or is it just me? :D)...

Damjan94 · 2021-01-30T21:42:08Z

src/assets/ui/uicontroller/uicontroller.gd

@@ -127,3 +130,16 @@ func get_child_node_names() -> Array:
 	for i in get_children():
 		name_list.append(i.name)
 	return name_list
+
+func pre_ins(ui_name: String):


why not do

func pre_ins(ui_name: String): instance_ui(ui_name) close_ui(ui_name)

Damjan94 · 2021-01-30T22:03:56Z

src/assets/ui/tasks/container/container.gd

+	GameManager.connect('state_changed', self, '_on_state_changed')
+
+
+func _process(_delta):#Called every frame to check interaction status


is there no way to change this to be event based?

nicemicro

I have not checked the codebase yet, I trust @Damjan94 that he did a thorough job and I'll look at it after his recommendations have been implemented.
However to expand on the bug @Damjan94 describes in their comment, it happens wit every item you pick up from the storage. Steps to reproduce:

take an item out of the storage.
walk away and drop the item on the ground.
try to pick up the item again
watch the item move to the center of the map
the item can not be picked up again.

Damjan94

if you add assert(picked_up_item == null) before this line, sometimes, when there's a lot of items around the container with the batteries, you end up with a perma item in your hand, that you can't drop.

actually, there don't need to be any items around the container. It sometimes will just put two items if it doesn't close the ui after you take away the first item, and you click to take away another item

Damjan94 · 2021-02-02T22:47:55Z

src/assets/items/item.gd

-	map_items.remove_child(self)
+	if item_from_container:
+		var path = get_tree().get_root().get_node(item_location)
+		path.remove_child(self)


the bug @nicemicro and I found(where items get teleported to (0,0) coordinate) is because item_from_container is never set to false. you could set it to false here...

nicemicro

I feel like the container.gd is doing too much with respect to synchronizing the data.
I would be more happy if we used an autoload like taskmanager.gd or rather a script attached to main.tscn to handle these kind of synchronization behaviors, as I would think in the future when we have multiple different tasks, having a common back end would be beneficial.

nicemicro · 2021-02-08T12:09:13Z

src/assets/player/player.gd

+	if can_pickup == true:
+		# Item Handler does not receive input as a child of a Viewport
+		item_handler._input(event)


the item_handler should do all the different types of checking whether it is possible to pick up an item or not.
Having a bool for this purpose is not good, because many different events can block pick up, and if two blocking events occur simultaneously, and one is over, that would maybe switch the bool back to true, even though the other blocking event still did not pass.
So instead of having a boolean, just add the additional conditions to item_handler._input().

nicemicro · 2021-02-08T12:18:08Z

src/assets/ui/tasks/container/container.gd

+
+func _process(_delta):#Called every frame to check interaction status
+	if not ui_data.empty():
+		rpc_id(1, "pass_data_server", ui_data)


I also don't understand, what happens if the ui_data becomes empty, when it was not empty before? does it not RPC the server in that case?
Moreover, when this RPC is called, the server will send the RPC to all clients, so it ends up setting it to itself again. Is this intended?

nicemicro · 2021-02-08T12:22:25Z

src/assets/ui/tasks/container/container.gd

+puppetsync func erase_children() -> void:#erases all child
+	for child in grid.get_children():
+		child.queue_free()
+
+puppetsync func reset() -> void:#Clears all the data
+	erase_data()
+	ui_data.clear()


shouldn't we check here whether the RPC is coming for the server?

Container refactor: adding the interaction resource to the map

…o container-tasks

renaming files

…suspect into container-tasks

added new items for the chemical cabinet

…suspect into container-tasks

added new items

…o container-tasks

zapzoop0099 added 3 commits January 30, 2021 18:44

Container task running good

8993e56

Merge branch 'main' of https://github.com/opensuspect/opensuspect int…

aed76a8

…o container-tasks

Removed test debug messages

8d62ec3

zapzoop0099 requested a review from a team January 30, 2021 13:54

Damjan94 requested changes Jan 30, 2021

View reviewed changes

nicemicro requested changes Jan 31, 2021

View reviewed changes

Damjan94 requested changes Feb 2, 2021

View reviewed changes

zapzoop0099 added 2 commits February 4, 2021 15:28

Fixed both bugs proposed by damjan94 and NiceMicro

04185c2

implemented rest of the changes

68548db

zapzoop0099 requested review from Damjan94 and nicemicro February 7, 2021 15:22

nicemicro requested changes Feb 8, 2021

View reviewed changes

nicemicro and others added 2 commits February 10, 2021 18:56

added the chemical cabinet and UI opening using the interaction resource

8d235b3

Merge pull request #1 from nicemicro/container-refactor

43bf38c

Container refactor: adding the interaction resource to the map

nicemicro linked an issue Feb 17, 2021 that may be closed by this pull request

Tasks system #196

Closed

zapzoop0099 and others added 14 commits February 24, 2021 10:14

updated with main

71a40f3

Updated with main

02cd2b1

Fixed merge mistakes

409bc6e

Custom task resource

3530845

Merge branch 'main' of https://github.com/opensuspect/opensuspect int…

8d229fc

…o container-tasks

Sync work

a8b3f17

renaming files

8996940

Merge pull request #2 from nicemicro/container-help

58ea05c

renaming files

Fixed sync state

ca16166

Merge branch 'container-tasks' of https://github.com/zapzoop0099/open…

6bc7a25

…suspect into container-tasks

Some basic item_holding_dic added

3d35c41

added new items for the chemical cabinet

63a890d

Merge pull request #3 from nicemicro/container-help

7162681

added new items for the chemical cabinet

Interaction done

b1ec4d5

zapzoop0099 and others added 9 commits March 5, 2021 20:12

Merge branch 'container-tasks' of https://github.com/zapzoop0099/open…

422e28d

…suspect into container-tasks

Basic stacking of items

fc83e06

added new items

6d4f183

Merge pull request #4 from nicemicro/container-help

d3f3743

added new items

Basic scene setup

5233618

Merge branch 'main' of https://github.com/opensuspect/opensuspect int…

b9cfc4d

…o container-tasks

Cleanup

885e262

Merge branch 'main' of https://github.com/opensuspect/opensuspect int…

6175723

…o container-tasks

Working Container

e58b129

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Container tasks #293

Container tasks #293

zapzoop0099 commented Jan 30, 2021

github-actions bot commented Jan 30, 2021

Damjan94 left a comment •

edited

Loading

Damjan94 Jan 30, 2021

Damjan94 Jan 30, 2021 •

edited

Loading

Damjan94 Jan 30, 2021

Damjan94 Jan 30, 2021 •

edited

Loading

Damjan94 Jan 30, 2021

Damjan94 Jan 30, 2021

Damjan94 Jan 30, 2021

Damjan94 Jan 30, 2021

nicemicro left a comment

Damjan94 left a comment

Damjan94 Feb 2, 2021

nicemicro left a comment

nicemicro Feb 8, 2021

nicemicro Feb 8, 2021

nicemicro Feb 8, 2021

		GameManager.connect('state_changed', self, '_on_state_changed')


		func _process(_delta):#Called every frame to check interaction status

Container tasks #293

Are you sure you want to change the base?

Container tasks #293

Conversation

zapzoop0099 commented Jan 30, 2021

github-actions bot commented Jan 30, 2021

Damjan94 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Damjan94 Jan 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Damjan94 Jan 30, 2021 • edited Loading

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

nicemicro left a comment

Choose a reason for hiding this comment

Damjan94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicemicro 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

Damjan94 left a comment •

edited

Loading

Damjan94 Jan 30, 2021 •

edited

Loading

Damjan94 Jan 30, 2021 •

edited

Loading