-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RGL object fetching reworked #312
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Piotr Jaroszek <[email protected]>
Change to draft until #148 is merged |
|
||
void Start() | ||
{ | ||
sceneManager = FindObjectOfType<RGLUnityPlugin.SceneManager>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think each NPC class should reference the RGL SceneManager. Is there a problem if the call to SceneManager.RegisterRGL is called by the class that spawns the NPC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apologize for the late response. I see one main issue with this approach, which is that for what I know, NPCPedestrian.cs
does not have a spawning class when AWSIM is run without SSv2. For example in Nishinjuku scene, the pedestrian GameObjects are already present in the scene.
This of course can be resolved by having an additional script in the scene, which is given references to all pedestrian GameObjects and is responsible for registering them with RGL. Do you want us to implement such a solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apologize for the late response.
no problem!
In any case, I don't think we should add code that depends on other classes. If such additions are made in the future, there will be more and more code to add caches other than RGL to each component class, such as NPCs, which we do not want.
For example, the following workarounds can be suggested
- Have NPC Pedestrian spawn at the start of Simulation. (And the spawning class will do the RGL cache)
- Refer to the first placed NPC Pedestrian and add it to the RGL cache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood, we will implement changes not to have references to RGL within pedestrian classes.
Nice work 👍 |
The approach for the system looks as follows:
In a result, there are no frequent fetches of the whole level (
FindObjectsOfType
).Requires #148