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

Adding Scoreboard command to the info.lua #246

Merged
merged 15 commits into from
Aug 8, 2022

Conversation

tonitch
Copy link
Contributor

@tonitch tonitch commented Apr 11, 2022

Implementing /scoreboard as part of the #158
info.lua updated for an overview of the work to do

@tonitch
Copy link
Contributor Author

tonitch commented Apr 11, 2022

I just noticed that my ide removed all the indentation. I don't know if this is a problem

@12xx12
Copy link
Member

12xx12 commented Apr 11, 2022

Hi and welcome to the project.

About the whitespace:
The change itself is not that bad. But to keep this PR clean, pls move it to a new PR

@tonitch
Copy link
Contributor Author

tonitch commented Apr 13, 2022

This is basically it for the command, I will add Comments and more comprehensive error message then It will be good to merge I think

@bearbin
Copy link
Member

bearbin commented Apr 14, 2022

I'll review this tomorrow in depth. It looks good generally, but the documentation messages could do with some standardisation, and the commands could do with some changes to correspond better to the vanilla ones.

@tonitch
Copy link
Contributor Author

tonitch commented Apr 15, 2022

I used this as reference for the commands, I tried to be as accurate as possible with the commands but there was not the outputs of the commands so I made it a bit like I though

I will launch a minecraft world to test and documents the outputs more accurately, I just have no time this w-e, I will try to do this next week

@tonitch tonitch marked this pull request as draft April 25, 2022 09:04
@tonitch
Copy link
Contributor Author

tonitch commented Apr 25, 2022

I want to make sure the code is clean and follow the project's structure so I checked other files as cmd_effect.lua for instance.

But I don't understand, Why does the Response Variable is returned as this doesn't seem to be handle by cuberite but also, why should I return the content of cPlayer:sendMessage to Response as this function doesn't seems to return anything...

@mathiascode
Copy link
Member

Why does the Response Variable is returned as this doesn't seem to be handle by cuberite

It's used for the console command to print the response:

Core/cmd_effect.lua

Lines 67 to 69 in baf6a4e

function HandleConsoleEffect(Split)
return HandleEffectCommand(Split)
end

but also, why should I return the content of cPlayer:sendMessage to Response as this function doesn't seems to return anything...

There are additional functions that return the message when appropriate:

Core/core_functions.lua

Lines 72 to 97 in baf6a4e

-- If the target is a player, the SendMessage function takes care of sending the message to the player.
-- If the target is a command block or the console, the message is simply returned to the calling function,
-- which delivers it appropriately
function SendMessage(Player, Message)
if Player then
Player:SendMessageInfo(Message)
return nil
end
return Message
end
function SendMessageSuccess(Player, Message)
if Player then
Player:SendMessageSuccess(Message)
return nil
end
return Message
end
function SendMessageFailure(Player, Message)
if Player then
Player:SendMessageFailure(Message)
return nil
end
return Message
end

@tonitch
Copy link
Contributor Author

tonitch commented Apr 26, 2022

I am currently implementing the console side of the command but I have a problem... scoreboard by cuberite work as a per world basis. so when a player execute a scoreboard command I usually get the player's world to create the scoreboard... but when the console does it. what should I do?

tonitch added 3 commits April 26, 2022 18:02
The tags file is used by ctags for quick tag finding, but as this is
generate do on every small changes with `ctags -R .` this should not be
in the git repo
if gPlayer then
gPlayer:SendMessage(Objective:GetDisplayName() .. " -> " .. Objective:GetName() .. ": " .. get_key_for_value(criterias, Objective:GetType()))
else
LOG(Objective:GetDisplayName() .. " -> " .. Objective:GetName() .. ": " .. get_key_for_value(criterias, Objective:GetType())) -- TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering... is there a better way to send the result of a command (with multiple line) than doing it with LOG() ?

Copy link
Member

@mathiascode mathiascode Apr 28, 2022

Choose a reason for hiding this comment

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

This is how the /tps command does it:

Core/cmd_tps.lua

Lines 34 to 42 in baf6a4e

function HandleTpsCommand(Split, Player)
local Response = {}
table.insert(Response, "Global TPS: " .. GetAverageNum(GlobalTps))
for WorldName, WorldTps in pairs(TpsCache) do
table.insert(Response, "World \"" .. WorldName .. "\": " .. GetAverageNum(WorldTps) .. " TPS")
end
return true, SendMessage(Player, table.concat(Response, "\n"))
end

I don't remember if we have other commands in the new format that output multiple lines.

Edit: another one
https://github.com/cuberite/Core/blob/master/cmd_players.lua

Copy link
Contributor Author

@tonitch tonitch left a comment

Choose a reason for hiding this comment

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

I think I'm quite done ?!

@tonitch tonitch marked this pull request as ready for review April 26, 2022 20:18
Copy link
Member

@mathiascode mathiascode left a comment

Choose a reason for hiding this comment

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

Use PascalCase in general.

CONTRIBUTORS Show resolved Hide resolved
@tonitch
Copy link
Contributor Author

tonitch commented Apr 28, 2022

There we go, Ready again!

Copy link
Contributor Author

@tonitch tonitch left a comment

Choose a reason for hiding this comment

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

all done

@mathiascode mathiascode merged commit cf466b8 into cuberite:master Aug 8, 2022
@mathiascode
Copy link
Member

@tonitch Could you create a follow-up PR addressing some unhandled exceptions when invalid command arguments are provided?

@tonitch tonitch deleted the Cmd_Scoreboard branch October 7, 2022 07:33
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.

4 participants