GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Support for custom tablist header and footer by Kecerim24 · Pull Request #513 · feather-rs/feather

Conversation

Kecerim24 commented Jan 9, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 added 3 commits

January 8, 2022 00:30


          Initial commit

c016d23

added system for adding or changing tablist headers and footers.
To make the tablist prettier


          Send extra info to new players

35babb7

Tablist header and footer now in TablistHeaderFooter type
Resources now used to store TablistHeaderFooter


          Cargo fmt

a8fbd38

Iaiao reviewed

Jan 10, 2022

View reviewed changes

Comment thread

feather/common/src/events.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

feather/server/src/systems/tablist.rs Outdated

Comment on lines +15 to +18

		game.insert_resource(TablistHeaderFooter {
		header: "{\"text\":\"\"}".to_string(),
		footer: "{\"text\":\"\"}".to_string(),
		});

Iaiao Jan 10, 2022

Copy link

Copy Markdown

Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 Jan 18, 2022

Copy link

Copy Markdown

Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Iaiao Jan 18, 2022

Copy link

Copy Markdown

Contributor

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 Jan 19, 2022

Copy link

Copy Markdown

Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Iaiao Jan 19, 2022

Copy link

Copy Markdown

Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 added 5 commits

January 18, 2022 22:01


          Use libcraft_text instead of String

e4507b1


          Cargo fmt

ba952b3


          Merge branch 'main' of https://github.com/feather-rs/feather into pre…

db7473a

…tty_tablist


          Cargo fmt

48f8f42


          Use default values for TablistHeaderFooter

22f2902

Add configurable values to config.toml

Iaiao reviewed

Jan 20, 2022

View reviewed changes

Comment thread

feather/server/config.toml Outdated

Comment on lines +16 to +17

		default_header = ""
		default_footer = ""

Iaiao Jan 20, 2022

Copy link

Copy Markdown

Contributor

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.

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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 and others added 2 commits

January 21, 2022 09:27


          Merge branch 'main' into pretty_tablist

486dd86


          Add default values for tablist header

7e0535f

Rename `default_header` to `tablist_header`
Rename `default_footer` to `tablist_footer`

Iaiao reviewed

Jan 21, 2022

View reviewed changes

Comment thread

feather/common/src/events.rs Outdated

Comment on lines +63 to +67

		#[derive(Debug, Clone)]
		pub struct TablistHeaderFooter {
		pub header: Text,
		pub footer: Text,
		}

Iaiao Jan 21, 2022

Copy link

Copy Markdown

Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 Jan 21, 2022

Copy link

Copy Markdown

Author

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Iaiao Jan 21, 2022

Copy link

Copy Markdown

Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Iaiao Jan 21, 2022

Copy link

Copy Markdown

Contributor

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

feather/server/src/systems/tablist.rs Outdated

		Ok(())
		}

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

Iaiao Jan 21, 2022

Copy link

Copy Markdown

Contributor

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.

It updates not only header, but also footer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

feather/server/src/systems/tablist.rs Outdated

		Ok(())
		}

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

Iaiao Jan 21, 2022

Copy link

Copy Markdown

Contributor

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.

It sends not only header, but also footer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 and others added 3 commits

January 21, 2022 20:45


          Move TablistHeaderFooter to quill/common/tablist.rs

45d01df

Rename `update_tablist_header` to `update_tablist_header_footer`
Rename `send_tablist_header_on_join` to `send_tablist_header_footer_on_join`


          Merge branch 'feather-rs:main' into pretty_tablist

3a96ed0


          Merge branch 'pretty_tablist' of https://github.com/Kecerim24/feather …

3c3f707

…into pretty_tablist

Kecerim24 commented Jan 21, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Iaiao suggested changes

Jan 26, 2022

View reviewed changes

Comment thread

feather/server/src/options.rs Outdated

Comment on lines +31 to +36

		/// The default tablist header.
		pub tablist_header: Text,

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

Iaiao Jan 26, 2022

Copy link

Copy Markdown

Contributor

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 Jan 26, 2022

Copy link

Copy Markdown

Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 Jan 26, 2022

Copy link

Copy Markdown

Author

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.

Nevermind, got it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Iaiao Jan 26, 2022

Copy link

Copy Markdown

Contributor

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.

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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kecerim24 and others added 3 commits

January 26, 2022 20:29


          Remove tablist_header and tablist_footer from Options

b0c2320

Add config to resources


          Merge branch 'feather-rs:main' into pretty_tablist

f6f8675


          Merge branch 'pretty_tablist' of https://github.com/Kecerim24/feather …

abe25d8

…into pretty_tablist

Iaiao suggested changes

Jan 26, 2022

View reviewed changes

Comment thread

feather/common/src/events.rs Outdated

Comment on lines +62 to +63

		#[derive(Debug)]
		pub struct TablistExtrasUpdateEvent;

Iaiao Jan 26, 2022

Copy link

Copy Markdown

Contributor

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.

#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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Move TablistExtrasUpdateEvent to quill/common/srs/events/change.rs

342dba2

Add docs

Iaiao approved these changes

Jan 26, 2022

View reviewed changes

Defman commented Jan 26, 2022

Copy link

Copy Markdown

Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Iaiao commented Jan 26, 2022

Copy link

Copy Markdown

Contributor

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)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

1 more reviewer

Iaiao

Iaiao approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Kecerim24 commented Jan 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tablist header and footer

Status

Description

Related issues

Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kecerim24 commented Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Defman commented Jan 26, 2022

Uh oh!

Iaiao commented Jan 26, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kecerim24 commented Jan 9, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Kecerim24 commented Jan 21, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.