-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Changed RPC strings to static functions #241
Conversation
…piler hits error if function doesnt exist, so typos are caught with static RPC functions)
## Normally, there would be a singleton which stores all players' peer_ids and player_ids, and the local peer id and player_id | ||
## Since there isn't, the player_id is calculated realtime by counting how many brawlers until our own... | ||
func get_player_id() -> int: | ||
var brawlers: Array[Node] = get_tree().get_nodes_in_group("Brawlers") |
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.
This won't work unfortunately - this class is released part of an addon, which can't rely on game-specific knowledge.
return latest_projectile_id | ||
|
||
## @experimental | ||
## Normally, there would be a singleton which stores all players' peer_ids and player_ids, and the local peer id and player_id |
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.
This also assumes a bit much about the addon user's game; see my comment on get_player_id.
Thanks for the PR! Could you please separate the static RPC call parts into its own PR? It's a good improvement and something I have considered myself too. This way we could discuss the projectile ID change separately. |
As for the projectile ID, I originally wanted to have it as EntityID. Same trick with using PlayerID. The problem is that PlayerID doesn't exist in netfox at all, yet all games use it in some way or another. Being a single variable which doesn't change, it would help tons. The full plan is to make a singleton (
Note this is a singleton, no need to have it autoload, could be a static class, as once it gets its initial state, its immutable pretty much. But ofc its the same as having it autoload pretty much. Having access to this singleton is quite handy, since the above data don't change, and are filled on joining. Having such a singleton would also solve the above problem with player_id being fetched in such a hacky/ugly way. What do you think? How would you like me to implement player id? The scope of whatever holds it must be above the brawler, so if not a singleton, then a Node named "LocalData" at the forest brawler scene (but I don't like having it on a scene, because if there is another playable scene loaded, it will conflict) |
8acc274
to
7a85290
Compare
Done 👍 |
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.
Added a version bump, otherwise looks good, thanks!
Solves #242(Related: #237 )also changed RPC strings to static functions
Strings in RPC calls dynamically search for RPCs.
By having them static, there is a slight performance gain (and ofc typos are caught by the compiler)