[3.x] Add NetworkedMultiplayerCustom so high-level multiplayer backends can be added from GDScript #63163
Conversation
dsnopek
force-pushed
the
multiplayer-peer-custom-3.x
branch
2 times, most recently
from
ab3d45c
to
c393507
Compare
There was a problem hiding this comment.
Really nice work 🏅 🏆 .
See my notes to clarify the method and signal.
I think this is really nice to have in 3.x and adds the functionality in a very self-contained way.
I think it should also be renamed to NetworkedMultiplayerCustom for clarity, but that's not so important given in 3.x every multiplayer peer has a different convention :)
dsnopek
force-pushed
the
multiplayer-peer-custom-3.x
branch
from
c393507
to
ac05c9d
Compare
|
@Faless Thanks for the review! :-) I've updated the PR per all your notes. Please let me know if there's anything else! |
dsnopek
changed the title
dsnopek
force-pushed
the
multiplayer-peer-custom-3.x
branch
from
ac05c9d
to
2339707
Compare
… be added from GDScript
dsnopek
force-pushed
the
multiplayer-peer-custom-3.x
branch
from
2339707
to
8ad5889
Compare
|
For some reason, changing the name from Anyway, none of the defaults actually changed, it's just representing them differently in the XML, where previously some child classes were marked as "overridding" the parent's default, now match the parent, and vice versa for other child classes. Hopefully, this is OK! Please let me know if you think it's worth putting some |
There was a problem hiding this comment.
Looks really good 🥇 , thanks!
I think this could go in for 3.5 or the first point release since it's completely self contained, just pinging @akien-mga and @Calinou about the doc change but I think it's just the ordering as dsnopek mentioned.
|
Thanks! |
| if (p_connection_status == ConnectionStatus::CONNECTION_CONNECTING && connection_status == ConnectionStatus::CONNECTION_DISCONNECTED) { | ||
| connection_status = p_connection_status; | ||
| } | ||
| // Can only go to connected, if we are connecting. |
There was a problem hiding this comment.
I was playing around with this today and got hung up on this bit. An error message or some actual documentation on this restriction would be good.
There was a problem hiding this comment.
Thanks! I've created a new PR adding those: #63628
|
Btw, this is a very cool class and I'm very excited about it. I've got Discord's Game SDK working with it and want to try EOS with it next. I loved that I was able to take my existing code that used the I have some more commentary to offer on the implementation/documentation. Would you prefer I just comment on the existing PR's or create some myself? |
|
@jarommadsen That sounds awesome! Please feel free to open your own PRs and ping me - that'd be great :-) |
…n to guide usage Some suggestions based on the new `NetworkedMultiplayerCustom` class introduced in godotengine#63163. @dsnopek I've made some suggestions to the docs based on what would have helped me through my first time. This in conjunction with godotengine#63628 and your new additions in godotengine#63637 should help new users understand what's going on without having to dig through the source code. I'd also like some help improving the `deliver_packet` method docs since I haven't gotten around to really digging through how that gets propagated. I assume the `from_peer_id` is meant to be the unique network id the sender of that packet was initialized with? And am I correct in assuming the `target_id`/`target_peer` is what it uses to deliver it the right place locally? I think some further documentation here would be helpful to know how to use `set_target_peer` in conjunction with this.
This is a proposed 3.x implementation for godotengine/godot-proposals#4873