-
Notifications
You must be signed in to change notification settings - Fork 3
Properly update Renderer.can_render
#38
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
Conversation
Renderer.can_render (Fixes #37)Renderer.can_render
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 do not think this solution is ideal as it couples the renderer to the manager and it will not work correctly without it, thus reducing flexibility. For example, I cannot make a custom renderer and have its can_render flag update, unless I store it as self.renderer in my bot or override the msg handler.
I suggest putting the can_render flag on the RLBotInterface and then have the managers and the renderer read that flag instead.
E.g. in the renderer:
@property
def can_render():
self._game_interface.can_render|
ah, probably a good idea |
|
Done, moved |
|
The server will respond with a status change message to confirm the change though, right? So you don't need the extra integration if you just update EDIT: Just checked, and no, the server never sends the RenderingStatus message under any circumstances. That seems like a bug. |
|
I was under the initial impression that render status updates were broadcast to all connections, not just the updated connection. This means that bots need there own filtering logic and the integration is required. If the update is only sent to the updated connection then you would be correct. The RenderingStatus message not being sent ever definitely seems like a bug. |
|
The relevant snippet: https://github.com/RLBot/core/blob/master/RLBotCS/Server/FlatBuffersSession.cs#L423-L435 My understanding is that it was supposed to be sent to the relevant process and possibly the match host (e.g. the GUI). The FlatbufferSession has to check the index anyway, so it would be easy to insert a message here. |
Fixes #37