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

Removed MOTD #4057

Merged
merged 3 commits into from
Apr 3, 2022
Merged

Removed MOTD #4057

merged 3 commits into from
Apr 3, 2022

Conversation

EPuncker
Copy link
Contributor

@EPuncker EPuncker commented Apr 1, 2022

Pull Request Prelude

Changes Proposed

GUILD MOTD HASN'T BEEN TOUTCHED!

For those who don't know what MOTD is, it was the small "Message Of The Day" window that appeared once a day before your character select window...

It was replaced in QT client by a new widget on client bottom, and it doesn't need any server-sided info:
image

I removed leftover code as it is not present in QT client anymore (since version 11?)

OTC can still be edited easily to have it client-sided only. https://github.com/edubart/otclient/blob/9c9b85ac5e9ae3cbacfc2d4993517c886d8881cd/modules/gamelib/protocollogin.lua#L186-L189

Issues addressed:
closes #4056

src/protocolstatus.cpp Outdated Show resolved Hide resolved
@EPuncker EPuncker added the decisions Debatable/disputable label Apr 1, 2022
@Zbizu
Copy link
Contributor

Zbizu commented Apr 1, 2022

protocolstatus can return "N/A" string

Copy link
Member

@DSpeichert DSpeichert left a comment

Choose a reason for hiding this comment

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

I'd leave the config option just to send it in XML and binary status protocol, at least while server lists use it.

@Zbizu
Copy link
Contributor

Zbizu commented Apr 2, 2022

I'd leave the config option just to send it in XML and binary status protocol, at least while server lists use it.

server lists are unlikely to back off considering that many servers are still running 8.6 and 10.98

@EPuncker
Copy link
Contributor Author

EPuncker commented Apr 2, 2022

re-review please 😄

Copy link
Member

@ranisalt ranisalt left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -160,7 +160,7 @@ void ProtocolStatus::sendInfo(uint16_t requestedInfo, const std::string& charact

if (requestedInfo & REQUEST_MISC_SERVER_INFO) {
output->addByte(0x12);
output->addString(g_config.getString(ConfigManager::MOTD));
output->addString("N/A"); //MOTD
Copy link
Contributor

Choose a reason for hiding this comment

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

// MOTD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't worry, @ranisalt clang will fix that in the future I hope

@DSpeichert
Copy link
Member

I'd leave the config option just to send it in XML and binary status protocol, at least while server lists use it.

server lists are unlikely to back off considering that many servers are still running 8.6 and 10.98

That's why I think it's a feature to still be able to configure MOTD - as long as serverlists continue to show it, it's better to put something in there than just "N/A".

@EPuncker
Copy link
Contributor Author

EPuncker commented Apr 3, 2022

I'd leave the config option just to send it in XML and binary status protocol, at least while server lists use it.

server lists are unlikely to back off considering that many servers are still running 8.6 and 10.98

That's why I think it's a feature to still be able to configure MOTD - as long as serverlists continue to show it, it's better to put something in there than just "N/A".

nooo, it was a client feature, not otservlist feature, they can start to parse the new "MOTD" from server login.php now, it have lots of useful info: boosted creature id, players online, twitch viewers, event schedule etc

@DSpeichert DSpeichert merged commit c8e8ebf into otland:master Apr 3, 2022
@LOLlolgrb
Copy link

It's still used by every single otlist, otclient is still using it. So why would it be removed?!? Jesus christ this repository has really fallen off the last couple of years...

Can we also update the readme to say that we're aiming to be a copy of tibia - not an open source mmorpg emulator and that you need to use the real tibia client to connect these days? That seems more accurate these days.

@EPuncker EPuncker deleted the bye-motd branch April 3, 2022 14:51
@EPuncker
Copy link
Contributor Author

EPuncker commented Apr 3, 2022

It's still used by every single otlist, otclient is still using it. So why would it be removed?!? Jesus christ this repository has really fallen off the last couple of years...

Can we also update the readme to say that we're aiming to be a copy of tibia - not an open source mmorpg emulator and that you need to use the real tibia client to connect these days? That seems more accurate these days.

if you are not happy, you can fork the repository and revert the patch, or not apply it to your current branch.

@LOLlolgrb
Copy link

It's still used by every single otlist, otclient is still using it. So why would it be removed?!? Jesus christ this repository has really fallen off the last couple of years...
Can we also update the readme to say that we're aiming to be a copy of tibia - not an open source mmorpg emulator and that you need to use the real tibia client to connect these days? That seems more accurate these days.

if you are not happy, you can fork the repository and revert the patch, or not apply it to your current branch.

If you don't want any users, keep your great work up.

The beauty with this PR is that we can finally see what servers are using TFS 1.5. Let me create a server list in a year that filters with this new dumb MOTD message, and let's see how many servers are actively using this distro, it won't be many.

@ranisalt
Copy link
Member

@LOLlolgrb I am really eager to see your results, please ping me in the future.

@otland otland locked as off-topic and limited conversation to collaborators Apr 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
decisions Debatable/disputable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[discussion] MOTD is gone from client
7 participants