search

darksoil-studio / p2p-shipyard

Ship cross-platform p2p apps
http://darksoil.studio/p2p-shipyard/
11 stars 3 forks source link

fix: don't require connection to signal server on launch #48

Closed mattyg closed 3 days ago

mattyg commented 1 week ago

I don't think it makes sense to require connectivity to the signal server on launch.

If the user launches the app without a network connection, then enables a network connection later, I don't want them to be restricted to their LAN until they restart the app.

mattyg commented 1 week ago

@guillemcordoba what do you think?

guillemcordoba commented 1 week ago

Well then this means that the default behavior of "oh I am disconnected from the internet, switch to LAN only connection" is not present at all. This disables demos for downstream apps in crowded events for example... Mmm are you saying that this should be handled by the consuming app by passing None wan_network_config?

mattyg commented 1 week ago

Ok I think I'm a little confused and I haven't dug into the holochain code to understand this. It looks like we're adding both WAN signal+bootstrap and LAN signal to the transport_pool config here https://github.com/darksoil-studio/p2p-shipyard/blob/5d04b15116d1f5748a7bbab9d6eed9a2166571d1/crates/tauri-plugin-holochain/src/config.rs#L58

Can holochain support both simultaneously? Or is it picking one or the other internally?

Well then this means that the default behavior of "oh I am disconnected from the internet, switch to LAN only connection" is not present at all. This disables demos for downstream apps in crowded events for example... Mmm are you saying that this should be handled by the consuming app by passing None wan_network_config?

Yeah I would rather p2p-shipyard just uses whatever is passed into wan_network_config so its clear to the app dev. Otherwise we could add an setting in p2p-shipyard config disable_wan_network_if_unreachable: bool or something like that just to make it explicit.

I'm not sure how it affect demos at events -- at an event a user would connect to WAN unless they put their phones in airplane mode during app launch and then re-enabled networking after. If holochain only supports one at a time, then I'm thinking either the app should be built for local-only, or it should expose a way to change the settings and reboot the app with new settings. Does that make sense?

guillemcordoba commented 3 days ago

Well I was trying to have the best UX possible given the constraints I have. This means that the plugin checks whether WAN is available, and if it's not, it switches to LAN only capabilities. This is in waiting for holochain to add proper LAN support. Holochain can support an array of signal urls, but when calling AdminWebsocket::agent_info it unfortunately only returns one. So for now we're stuck in either WAN mode or LAN mode yes.

I would much rather have some config like switch_to_lan_only_if_wan_is_unreachable as part of HolochainPluginConfig and have that default to true. I'm trying to deliver the best experience possible to devs that may not fully understand the internals of holochain, so I want the best behavior possible to be the default and the complicated custom config to be opt-in. Would this solution suffice?

mattyg commented 3 days ago

I would much rather have some config like switch_to_lan_only_if_wan_is_unreachable as part of HolochainPluginConfig and have that default to true. I'm trying to deliver the best experience possible to devs that may not fully understand the internals of holochain, so I want the best behavior possible to be the default and the complicated custom config to be opt-in. Would this solution suffice?

Yeah what if the config is an enun with Wan, Lan, and PreferWanIfReachable or something like that, and document that its pinging the signal server to determine connectivity on app startup. The default would be PreferWanIfReachable.

I can make a PR for it if that works for you.

guillemcordoba commented 3 days ago

Mmm but then that conflicts with the wan_network_config that's the second parameter in HolochainPluginConfig::new...

mattyg commented 3 days ago

Mmm but then that conflicts with the wan_network_config that's the second parameter in HolochainPluginConfig::new...

Ah I see, maybe we add a field to the wan_network_config fallback_to_lan: bool or something similar. Then we're clear and explicit that the dev is choosing either lan, wan, or wan that falls back to lan.

guillemcordoba commented 3 days ago

I like the fallback_to_lan field. I'll add that.

guillemcordoba commented 3 days ago

Implemented in https://github.com/darksoil-studio/p2p-shipyard/pull/55