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

Support for custom tablist header and footer #513

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

Kecerim24
Copy link

@Kecerim24 Kecerim24 commented Jan 9, 2022

Tablist header and footer

Status

  • Ready
  • Development
  • Hold

Description

Created a new TablistHeaderFooter that is sent to newly connected players. Other code can update this by changing this structure in Game::Resources and causing a TablistExtrasUpdateEvent which resends the data to all clients.

Related issues

Checklist

  • Ran cargo fmt, cargo clippy --all-targets, cargo build --release and cargo test and fixed any generated errors!
  • Removed unnecessary commented out code
  • Used specific traces (if you trace actions please specify the cause i.e. the player)

Kecerim24 added 3 commits January 8, 2022 00:30
added system for adding or changing tablist headers and footers.
To make the tablist prettier
Tablist header and footer now in TablistHeaderFooter type
Resources now used to store TablistHeaderFooter
feather/common/src/events.rs Outdated Show resolved Hide resolved
Comment on lines 15 to 18
game.insert_resource(TablistHeaderFooter {
header: "{\"text\":\"\"}".to_string(),
footer: "{\"text\":\"\"}".to_string(),
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if you add the initial values to config.toml

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's that important to have default values, because this isn't a vanilla feature. If anyone needs this, it would be better to use a plugin.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feather is not even close to be fully survival-compatible, so the only usage for Feather in production will be lobbies and mini-games. And there's 99% chance that everyone needs this feature. If someone wants more advanced features (like placeholders, animations), it would still be possible to override the resource by a plugin. I don't know why vanilla server doesn't have this feature, but lack of feature is not a good thing to copy.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then in what format and where in the config would you like it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what format

Just like motd, using Text::from(str). Or there is libcraft_text::markdown module that allows writing rich text components in format @red some text @on_hover @show_text @green some hover text, but I don't see any existing usage or documentation for this format.

Where

[server] section

Comment on lines 16 to 17
default_header = ""
default_footer = ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users won't understand what header/footer is, I think it should be named like tablist_header or playerlist_header, and have default values to make it even more obvious (like vanilla's "A Minecraft Server" default motd).

Kecerim24 and others added 2 commits January 21, 2022 09:27
Rename `default_header` to `tablist_header`
Rename `default_footer` to `tablist_footer`
Comment on lines 63 to 67
#[derive(Debug, Clone)]
pub struct TablistHeaderFooter {
pub header: Text,
pub footer: Text,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feather_common::events is for events, while TablistHeaderFooter is a resource. I'm not sure what file would fit best for this small struct, but definitely not events.rs

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does it need to be for plugins to get to it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quill/common. Quill doesn't support resources though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can create feather/common/src/tablist.rs or quill/common/src/tablist.rs, but not in feather/server. No other crate can access feather-server.

@@ -63,6 +75,34 @@ fn add_tablist_players(game: &mut Game, server: &mut Server) -> SysResult {
Ok(())
}

fn update_tablist_header(game: &mut Game, server: &mut Server) -> SysResult {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It updates not only header, but also footer.

Ok(())
}

fn send_tablist_header_on_join(game: &mut Game, server: &mut Server) -> SysResult {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sends not only header, but also footer.

Kecerim24 and others added 3 commits January 21, 2022 20:45
Rename `update_tablist_header` to `update_tablist_header_footer`
Rename `send_tablist_header_on_join` to `send_tablist_header_footer_on_join`
@Kecerim24
Copy link
Author

Kecerim24 commented Jan 21, 2022

(45d01df) quill/common/src/tablist.rs of course

Comment on lines 31 to 36
/// The default tablist header.
pub tablist_header: Text,

/// The default tablist footer.
pub tablist_footer: Text,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have Config for server config and Options for Server options (mostly for initial connection handling, status, ping, working with proxies), so I think tablist values should only be in Config.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let server_options = game.resources.get::<Server>().unwrap().options.clone();

game.insert_resource(TablistHeaderFooter {
    header: server_options.tablist_header.clone(),
    footer: server_options.tablist_footer.clone(),
});

Then how do I get it from config to use it for TablistHeaderFooter in tablist.rs.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, got it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass it to register(game) function, or even better, make Config (or even Rc<Config> to make it immutable) a resource and Options non-resource. Options is never used as a resource (just did a quick search on cs.github)

Comment on lines 62 to 63
#[derive(Debug)]
pub struct TablistExtrasUpdateEvent;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#522 is now merged, so quill-compatible events should be in quill/common/srs/events/change.rs, implement Serialize, Deserialize, Clone, be defined in quill/common/src/component.rs in HostComponent enum and have bincode_component_impl! in the end.

@Defman
Copy link
Member

Defman commented Jan 26, 2022

I'm not sure if this should be store in the default config. It would be quite easy to write a plugin, which sets the header and footer based on a config file. Going down this rapid hole, would result in a masive config file.

@Iaiao
Copy link
Contributor

Iaiao commented Jan 26, 2022

It's currently impossible to write a plugin that interacts with resources, and we don't have a config API yet. Also, I think it would be a great first experience if we have static tablist header/footer built-in, the same way as we do with motd. Like I said in the previous conversation, everyone wants a pretty tablist in their [mini-game or lobby] server, and it's important to have very-commonly-used features built-in rather than to rely on third-party "must-have" plugins (which we don't even have yet).
btw rust's stdlib sometimes gets replacements for very popular crates (plugins) like assert_matches (which is going to land soon) or lazy_static (which was superseded by once_cell and now we have #![feature(once_cell)] in std::lazy)

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

3 participants