Official Allow multiple entry/exit points in Locations

Hello everyone, my name is George. I would like to work on card number 195 titled the same as this thread. It says it is a good first task, and I am confident I can complete it.

Edit: Link to the card
https://open.codecks.io/unity-open-...allow-multiple-entry-exit-points-in-locations

 

Will be good to discuss the your ideas before you begin, based on good traditions.

P.S>
I think good style to add to your post the blue mark Official, it can be set after creation thread also.

 

Creating this post does not "assign" a task to you, as community work isn't officially "assigned" to one person. Anyone is allowed to work on a community card to give as many people the opportunity to contribute as possible, so feel free to fork the repository and create your own PR. Excited to see how you tackle this.

While I can agree that explaining implementation is a good idea, what I've seen lately is that forum threads for cards mainly discuss implementation for complex problems and not straightforward first tasks like this.

Like I said in the other thread, @cirocontinisio is the one that usually gives the "Official" mark, and for this particular task, it's not really necessary because it's a straightforward "good first task" that shouldn't involve too much implementation discussion.

For example, there was a good first task a little while ago that Ciro shared a video of how to implement in GitHub comments, and no forum thread was created. I waited a few days to see if the people in that GitHub discussion would get to it, and when they didn't, I took a crack at it and completed it without creating a new forum thread saying I would, just a PR when the changes were in. From that experience, it seems to me that "good first tasks" aren't meant for collaboration and really just a way to get people the positive feeling of having a contribution to the project to build confidence to help with something more complex in the future.

 

I do not know how to do the blue mark thing.

It seems from the card they want the player to be moved to a specific spawn point in another scene when triggered.

I was thinking about having a scriptable object represent the spawn points(name, scene name, location, rotation). A simple script would handle the scene change and would take an object as reference. Other scripts(like a trigger volume, dialogue manager, etc) would give that script their point reference and ask it to execute the load. When the scene loads the character controller and camera move themselves to the needed location and rotation.

This is my initial ideas on this, but I expect there to be minor changes as I start working on it(if cleared to do so).

 

Based on shuttle's post I am going to start work on this.

 

For sure, go right ahead and do an implementation. This is supposed to be a learning project, so what better way to learn than by doing?

There have been many people that made their first post ever (like you) to ask to be assigned a task on this project, and without a response, those people haven't been heard from on the forums again. It is great to ask for permission, but on something like this, the worst that could come of someone creating a PR to address an issue "without permission" is that it doesn't get accepted, but at least that person would have the experience of doing the implementation.

To tell another story, there was a discussion about making it easier for people to contribute on Discord a few weeks ago, and a lot of it was just not knowing that it was okay to try something. The other big thing was just the sheer overwhelming complexity of the project itself, which is why Ciro has been really great at trying to classify tasks as green/yellow/red for complexity. The only way someone new is going to ramp up is to start small, get some easy wins to build confidence, and then who knows, maybe one day soon contribute a bigger feature.

 

If you are the author of thread, you can edit the thread. In the upper part (in the Edit mode) of your thread will be link Thread Tools, from it you can select the Edit Title of this thread and set thread prefix.

You can get started nothing can stop you, the @shuttle127 (and I ) is very serious

But you must know - the main aim of this project it's a learning the good style Unity programming. Which achieved by discussion different ideas and selection the best (optimal) solution. And if you want, that your realization will be finale realization which included implement all good ideas. Will be good to discuss it, before you spent to it many time, because exist possibility, that somebody give more optimal solution and the Community decide that it more appripriate for this project.
But if you ready to produce "some prototype" which after it will discusse by Community , not spending many your time. it's also ok.

 

I hope you see Game Architecture Overview and Event system (from Wiki) which is the base for Scene Loading in this project.

 

I noticed those things after looking at the code(after I made that post), and I intended to tailor my solution to fit with what already existed. Still thank you very much for showing me to the wiki. That is some very useful information.

 

Hey @georgefnix, welcome to the project!

I don't think regular users can mark threads as Official, only admins should be able to. I'll set it as official.

 

I checked it, It's possible now, @cirocontinisio May be good idea to block it and make corresponding note regarding creation of threads for cards from road map

 

Thanks @cirocontinisio . Sorry for the delay, yesterday was trying to understand the systems and scripts that are already in place and working on a solution that fits within the current systems.

At the moment, but this may change:
The Scriptable Object representing the location(name TDB) is used in a new event the LocationLoader is listening to(I am not going to remove anything that already exists). The location loader does its normal thing, but once all of the scenes are loaded it raises a new event that communicates where to move the protagonist. Either the spawner or the protagonist(TBD) are listening and move to that new location. The exit code will have to be changed to have a parameter that accepts a SO representing location.

Edit: Amel, I read your post. That is very interesting and I will give it thought. I didn't consider the use of scriptable objects in that way.

 

Currently the loader accepts multiple scenes from an array to be loaded additively. It sets the first element of this array to be the active scene. Despite there being an active scene, it appears the other scenes are still being rendered. In the attached image(which is supposed to be beach) you can see forest's trees as well as significant z fighting of the ground textures.

I am uncertain how this should be addressed. Either we do not load multiple scenes, or we disable the non-active scene elements(or some similar solution I am yet unaware of).

The other object I am contemplating. I am considering using the locationSO for my task's purpose. To wit, I would need to add two Vector3's to that object. This obviously changes what the SO was initially defined for(to delineate between scenes for menus and scenes for gameplay), but it is exactly descriptive of what the task requires. I was initially going to make another SO with a similar name(ex: localeSO or placeSO), but a designer could easily get confused which SO they need to create. I seek feedback if this matters.

 

Thank you Amel. I made quite a few changes based upon your input especially.

Regardless, this is my PR. It also covers card 197 since that uses the changes to card 195(and it was a small addition to my other work).

Edit: Based on feedback from my original PR I made an updated PR.
It can be found here:
https://github.com/UnityTechnologies/open-project-1/pull/269

I also made a video demonstrating my PR. It may not be done processing by the time you see this.

 

Quick reply after first look:
You use term the Location for points of spawn on scene? It's not good, because currently "the Location in Editor" it's a some special type of SO.
IMHO it is more appropriate to change his name to something else - Scene point / spawn point and that kind of thing

 

It isn't just points of spawn on scene. It is points in a scene that can be used for anything including spawning a character(or teleporting, or whatever). For example, if an npc wants to walk to the clocktower they could query that location and begin pathing. In my opinion, in the context of video games a scene could represent a location, but in many cases it would represent an area(that has locations, ex: a town that has a market, bank, bridge, etc).

If it is important to the team I am not completely opposed to changing the names of the scripts.
Currently:
Scenes are represented by SceneSO
Locations in those Scenes are represented by LocationSO

If SceneSO is to be changed to LocationSO. Then LocationSO should be named something that isn't a synonym of location(like place). PointOfInterestSO or PointSO would be my suggestion.

Update: I am in the process of changing them to my second suggestion(which happens to be one of DSivtsov's and is more general). It is a longer task then I originally envisioned as I have more to rename than the classes.

Update 2: Changes made. All instances(I could find) of Scene is now Location, and every Location is Point.

 

After reading the other thread(about card 197) and some thought, I have decided to remove my request to merge. The other card is closely related to this one(that is why a two additional scripts to 195 solved 197 for me), and because I changed aspects of how things were structed I feel like my solution steps on too many toes.

I do not regret this at all. I learned a lot, especially that I need to improve my communication. Thank you all. I will probably be back at some point with an improved process.

 

I took yesterday off, and with that time I realized that I still understand this problem well and I can contribute in a way that doesn't mess with the way things are done. In this pull request I tried to change as little as possible.

Pull Request: https://github.com/UnityTechnologies/open-project-1/pull/275

PS: Thank you for the likes and responses to this thread.

 

Will take a look at it soon!

 

Edit: I have created a PR of my approach
PR: https://github.com/UnityTechnologies/open-project-1/pull/284
How to use: https://imgur.com/a/kEiDOZm

---

Hi all, I've been studying @georgefnix's PR and thought it can be improved to avoid putting additional responsibility on the Protagonist class to teleport to its spawn position.

Since the SpawnSystem already includes provisions for defining multiple Spawn Locations, perhaps we can just add enhancements to pick an appropriate Spawn Location, based on data passed by the LocationExit taken by the Protagonist.


I'm still getting around the architecture, but I'm currently trying this solution:

1. We create "paths" to link location entrances and exits using ScriptableObjects. Each path is just an SO asset with a sensible name (ex. "BetweenBeachAndForest", "BetweenBeachAndTestingGround").
2. Each LocationExit is assigned a path (ex. the ExitToForest in the screenshot above would be assigned the "BetweenBeachAndForest" path). Once the LocationExit is triggered, it saves the PathTaken to a runtime anchor.
3. Add new Spawn Locations to the SpawnSystem as above. Spawn Locations that represent entrances coming from a path will have a LocationEntrance with a path assigned to it.
4. When the next location loads and SpawnSystem starts, it checks the PathTaken runtime anchor.

- If a path is not set, spawn using the Default Spawn Index.
- If a path was set, find and use the Spawn Location with a LocationEntrance whose path == PathTaken. If not found, fallback to Default Spawn Index.
​

*EntranceFromTestingGround is unused because the current TestingGround scene doesn't actually have an exit leading to the Beach scene yet.

The downside to this approach is the SpawnSystem will become very tightly coupled to this responsibility. But the good thing is that SpawnSystem can still fallback to its default spawn location ("Location 01") and continue to work as before. Also, each Location doesn't need to know anything else about other locations, other than the Path that links between them. And entrance locations can also be visualized while editing the scene, as above. Let me know what you think about this approach so far.

Prior to this, I was exploring the following:
- An FSM or some kind of game manager could be used to determine the location entrances, but it seemed overkill given the early state of the game and could be difficult to change for testing purposes.
- PathTaken could be passed via the LoadLocation event channel, but it wouldn't make sense when loading menus.

 

It's so much more valuable to have such video, that goes over the changes, than just a PR! So thank you for recording it

 

Does this now count as completed? Was looking at the card and wanted to work on a solution but it looks like it is solved now.

 

It's definitely "in" in the sense that we have a working solution, but I don't think it's fully complete. And it can always be refined.
So I wouldn't suggest to create another different solution, unless you see something broken with the current one that can't be fixed.

 

Fair enough, I figured as much.
Will take a look at the possibility of making the entry/exit location module even better.

 

Hi Amel, thanks for introducing the convenience features to the system. However, I noticed that it also broke the SpawnSystem's original ability to be usable as a default spawn location when no other spawn location exists. I've created an Issue and PR for this.

Issue: https://github.com/UnityTechnologies/open-project-1/issues/333
PR: https://github.com/UnityTechnologies/open-project-1/pull/334

Hope this helps for the meantime.

 

I also identified it but thought it is OK. The part that is getting the Game objects with tag is returning in an order where the Location 01 spawn point is at the last of the returned list, top two are entrances from locations.

 

In scenes that don't have other spawn locations, ex. TestingGround_Small, the player wouldn't spawn and it would throw this error: "Exception: No spawn locations set." The fix I pushed should allow SpawnSystem to work as the default spawn location again.

I remember the game was being designed to allow any scene to be quickly tested by opening it and directly pressing play on the editor. The recent commit id# 6d2c22a kind of broke that, so I figured a fix is necessary.

Also, the fix will guarantee that when a scene containing other spawn locations (ex. Beach) is played by directly pressing play on the editor, the player will spawn at the position of SpawnSystem instead of a random spawn location. I believe this behavior makes it clear that the player has entered the location without a PathTaken, or that none of the other entrances matched the PathTaken. This could be useful for spotting unintended entrance/exit path configuration errors in the future.