Skip to content
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

Get the protocol version to send from the ping packet #20

Merged
merged 7 commits into from
Dec 1, 2018

Conversation

iceiix
Copy link
Owner

@iceiix iceiix commented Nov 6, 2018

@iceiix
Copy link
Owner Author

iceiix commented Nov 6, 2018

This basically works, but there is an implementation problem with saving/getting the version - you have to click refresh to connect with the right version, because it is retrieved here:

                // TODO: move getting protocol version out of refresh
                let server_protocol_versions = self.server_protocol_versions;
                backr.add_click_func(move |_, game| {
                    let protocol_version: i32 = *server_protocol_versions.get(&address).unwrap_or(&protocol::SUPPORTED_PROTOCOL);
                    println!("self.server_protocol_versions.get = {:?}", self.server_protocol_versions);
                    game.screen_sys.replace_screen(Box::new(super::connecting::Connecting::new(&address)));
                    game.connect_to(&address, protocol_version);
                    true
                });

trying to fix this, but if I do something like:

                let server_protocol_versions = self.server_protocol_versions;
                backr.add_click_func(move |_, game| {
                    let protocol_version: i32 = *server_protocol_versions.get(&address).unwrap_or(&protocol::SUPPORTED_PROTOCOL);
                    println!("self.server_protocol_versions.get = {:?}", self.server_protocol_versions);
                    game.screen_sys.replace_screen(Box::new(super::connecting::Connecting::new(&address)));
                    game.connect_to(&address, protocol_version);
                    true
                });

then:

error[E0495]: cannot infer an appropriate lifetime due to conflicting requirements
   --> src/screen/server_list.rs:156:38
    |
156 |                   backr.add_click_func(move |_, game| {
    |  ______________________________________^
157 | |                     let protocol_version: i32 = *server_protocol_versions.get(&address).unwrap_or(&protocol::SUPPORTED_PROTOCOL);
158 | |                     println!("self.server_protocol_versions.get = {:?}", self.server_protocol_versions);
159 | |                     game.screen_sys.replace_screen(Box::new(super::connecting::Connecting::new(&address)));
160 | |                     game.connect_to(&address, protocol_version);
161 | |                     true
162 | |                 });
    | |_________________^
    |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 104:5...
   --> src/screen/server_list.rs:104:5
    |
104 | /     fn reload_server_list(&mut self,
105 | |                           renderer: &mut render::Renderer,
106 | |                           ui_container: &mut ui::Container) {
107 | |         let elements = self.elements.as_mut().unwrap();
...   |
311 | |         }
312 | |     }
    | |_____^
    = note: ...so that the types are compatible:
            expected &mut screen::server_list::ServerList
               found &mut screen::server_list::ServerList
    = note: but, the lifetime must be valid for the static lifetime...
note: ...so that the type `[closure@src/screen/server_list.rs:156:38: 162:18 server_protocol_versions:std::collections::HashMap<std::string::String, i32>, address:std::string::String, self:&mut screen::server_list::ServerList]` will meet its required lifetime bounds
   --> src/screen/server_list.rs:156:23
    |
156 |                 backr.add_click_func(move |_, game| {
    |                       ^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0495`.

need to find out how/where to save/load the server to protocol version map, so it can be used on connect.

@iceiix
Copy link
Owner Author

iceiix commented Nov 6, 2018

Some good news: 1.11 is similar enough to 1.10.2 that this change allows Steven to at least connect to both server versions, even without changing the packets (#19 - though not all packets may be processed correctly)

@iceiix
Copy link
Owner Author

iceiix commented Nov 7, 2018

Forcing a server list refresh works, but isn't a good solution because it breaks the server status text, stuck on "Connecting...": - even though connecting works:

screen shot 2018-11-06 at 6 00 15 pm

@iceiix
Copy link
Owner Author

iceiix commented Nov 9, 2018

The server list is loaded from disk (servers.json), not memory, so the problem arises where to get the ping version from, since the closure is passed to .add_click_func(move |_, game| { when initializing the server list. The server connection is initiated in game.connect_to(&address), so maybe that function could retrieve the version, stashed in the Game object, but can tick() access the game object?

@iceiix iceiix force-pushed the version_from_ping branch 2 times, most recently from 6191652 to 8630338 Compare November 28, 2018 01:32
@iceiix
Copy link
Owner Author

iceiix commented Dec 1, 2018

Serializing to disk for now (server_versions.json), although not necessarily ideal, this matches the pattern used by src/screen/edit_server.rs (writes to servers.json) and src/screen/server_list.rs (reads servers.json). This also reduces the likelihood that the user will attempt to connect to a server before receiving the ping response, since it is persisted between application launches (although it may still be possible albeit unlikely, if this situation occurs a warning is logged).

@iceiix iceiix merged commit d80eca3 into updates Dec 1, 2018
@iceiix iceiix deleted the version_from_ping branch December 1, 2018 00:41
iceiix added a commit that referenced this pull request Dec 1, 2018
Only a minor update, -1 now indicates no color, so changed u8 to i8:

https://wiki.vg/Protocol_History#16w50a
https://wiki.vg/index.php?title=Protocol&diff=8543&oldid=8405
https://wiki.vg/index.php?title=Protocol&oldid=8543

and updated version numbers. 1.11.2 uses the same 1.11 assets, which can
be found by looking up 1.11.2 in:

https://launchermeta.mojang.com/mc/game/version_manifest.json
https://launchermeta.mojang.com/v1/packages/6bd228727ed48bd7ac7bdc0088587dad0fb7c02b/1.11.2.json

1.11.2/1.11 is compatible except for the version number, which is now sent matching the server (#20), so no backwards-compatible branch for 1.11 (315) is needed.

https://github.com/iceiix/steven/issues/18 Enhance protocol support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant