vraibae / IntermediateUnityClass2023

MIT License
0 stars 1 forks source link

Feedback #1

Open Wadarass opened 1 year ago

Wadarass commented 1 year ago

https://github.com/vraibae/IntermediateUnityClass2023/blob/c2bf24ef9a4ed0ac595511242b7f03d7babdd5a4/GameheadsDialogue/Assets/Scripts/DialogueStarter.cs#L22-L29

So this looks like it does what I asked so that's good. However, it's not using the interactable system from last years class (https://github.com/GameheadsClass/InteractableClass2022). That way you wouldn't need the Button component. The interaction system would handle all of that. The interactable system had a FPS controller and items placed in a 3d world...so as you walk up to them the items do different things.

Also, you are getting a reference to the dialogue manager from this class. In a game with many items that could trigger dialogue that wouldn't work as there is only 1 dialogue manager (it's a singleton). But since the dialouge manager is a singleton you can call the startdialogue function through the singleton interface instead of attaching it directly to this component.

Also you also directly reference the dialogue line in this component. You probably wouldn't do that in a real game. You would just reference the dialogue line by name and the manager would look it up from the loaded database.

vraibae commented 1 year ago

Oh darn. Didn’t realize the Interactable class was the way to go here. Duly noted, thank you for the feedback (and for the class)!

Is it always better to reference the component by name? I thought that it might be more prone to issues from typos and such, so I actually changed my mind partway. On Apr 10, 2023 at 3:28 PM -0700, Raymond Graham @.***>, wrote:

https://github.com/vraibae/IntermediateUnityClass2023/blob/c2bf24ef9a4ed0ac595511242b7f03d7babdd5a4/GameheadsDialogue/Assets/Scripts/DialogueStarter.cs#L22-L29 So this looks like it does what I asked so that's good. However, it's not using the interactable system from last years class (https://github.com/GameheadsClass/InteractableClass2022). That way you wouldn't need the Button component. The interaction system would handle all of that. The interactable system had a FPS controller and items placed in a 3d world...so as you walk up to them the items do different things. Also, you are getting a reference to the dialogue manager from this class. In a game with many items that could trigger dialogue that wouldn't work as there is only 1 dialogue manager (it's a singleton). But since the dialouge manager is a singleton you can call the startdialogue function through the singleton interface instead of attaching it directly to this component. Also you also directly reference the dialogue line in this component. You probably wouldn't do that in a real game. You would just reference the dialogue line by name and the manager would look it up from the loaded database. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

Wadarass commented 1 year ago

Is it always better to reference the component by name? I thought that it might be more prone to issues from typos and such, so I actually changed my mind partway.

It’s not always better but you definitely don’t want to directly reference the dialogue line. In a real game there would be some sort of tag or id that you would use to reference it. And that same tag would be used in the dialogue manager in the dictionary. In this case we just use a string cause it’s easy.

On Tue, Apr 11, 2023 at 5:51 PM Bae @.***> wrote:

Oh darn. Didn’t realize the Interactable class was the way to go here. Duly noted, thank you for the feedback (and for the class)!

On Apr 10, 2023 at 3:28 PM -0700, Raymond Graham @.***>, wrote:

https://github.com/vraibae/IntermediateUnityClass2023/blob/c2bf24ef9a4ed0ac595511242b7f03d7babdd5a4/GameheadsDialogue/Assets/Scripts/DialogueStarter.cs#L22-L29 So this looks like it does what I asked so that's good. However, it's not using the interactable system from last years class ( https://github.com/GameheadsClass/InteractableClass2022). That way you wouldn't need the Button component. The interaction system would handle all of that. The interactable system had a FPS controller and items placed in a 3d world...so as you walk up to them the items do different things. Also, you are getting a reference to the dialogue manager from this class. In a game with many items that could trigger dialogue that wouldn't work as there is only 1 dialogue manager (it's a singleton). But since the dialouge manager is a singleton you can call the startdialogue function through the singleton interface instead of attaching it directly to this component. Also you also directly reference the dialogue line in this component. You probably wouldn't do that in a real game. You would just reference the dialogue line by name and the manager would look it up from the loaded database. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/vraibae/IntermediateUnityClass2023/issues/1#issuecomment-1504351254, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVDQXX5CRDHRAFVRBHBWJTXAX4BVANCNFSM6AAAAAAWZPVCRY . You are receiving this because you authored the thread.Message ID: @.***>

-- Raymond Graham cell - 408-594-8620