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

Fix /level command's pattern #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Lymkwi
Copy link
Contributor

@Lymkwi Lymkwi commented Sep 11, 2015

  • Players can now use /level and /level without any troubles

@cornernote
Copy link
Owner

/level was also intended for admins to change the level of a player.

Eg, to move singleplayer to level 2:

/level singleplayer 2

Does this still work?

@Lymkwi
Copy link
Contributor Author

Lymkwi commented Sep 14, 2015

See my comment in #40 .

@cornernote
Copy link
Owner

Looking at the changelog for this PR, I can't see a /setlevel command. Am I missing something?

@Lymkwi
Copy link
Contributor Author

Lymkwi commented Sep 17, 2015

No you didn't miss anything, I just had not the time yet to work on that. I will code the /setlevel command later this day.

@cornernote
Copy link
Owner

The /level command worked fine to setlevel before your change, so it's just a matter of copying that back and renaming the command. Permissions should also be considered. All users should be able to check the players level, but a special permission is required to set the players level.

@cornernote
Copy link
Owner

Also, perhaps it should be /getlevel and /setlevel. And remove /level altogether. What do you think?

@Lymkwi
Copy link
Contributor Author

Lymkwi commented Sep 17, 2015

I think /level is just fine to get level, so is /getlevel, but /getlevel is more explicit in its name about what it does, so I will go with /getlevel and /setlevel. I'm working on it right now.

EDIT: Upated, but I had to fix a weird crash in feats whenever I entered the name of a player who existed, but was not connected Lymkwi@910a81c

 - Players can now use /getlevel and /setlevel without any troubles
 - Also fixed a crash in level set when player was not connected/data about them was not loaded
@cornernote
Copy link
Owner

@Lymkwi , this has a conflicting file. If you can re-make it i'll merge it in.

@Panquesito7
Copy link
Contributor

@Lymkwi , this has a conflicting file. If you can re-make it i'll merge it in.

You can fix the merge conflicts. Or, if you want I can re-make it later. 🙂

@Lymkwi Lymkwi closed this Jun 17, 2021
@Lymkwi Lymkwi reopened this Jun 17, 2021
@Lymkwi
Copy link
Contributor Author

Lymkwi commented Jun 17, 2021

(my bad for accidentally closing)
I'll see what I can do. This is code I wrote six years ago back in the 0.4.X days of MineTest. I'm not even sure my knowledge of the Skyblock API is up to date

@Panquesito7
Copy link
Contributor

@Lymkwi, if you want, I can re-work this PR and also give you credit for it. 🙂

@Lymkwi
Copy link
Contributor Author

Lymkwi commented Jun 18, 2021

@Lymkwi, if you want, I can re-work this PR and also give you credit for it. 🙂

To be fair that might take us less time if you do so. I'm not sure I'll have time and energy to go over this code and rework it immediately.
Ask me if you have a doubt about what something was supposed to mean/do. I should be able to tell from the old code.

@Panquesito7
Copy link
Contributor

@Lymkwi, if you want, I can re-work this PR and also give you credit for it. 🙂

To be fair that might take us less time if you do so. I'm not sure I'll have time and energy to go over this code and rework it immediately.
Ask me if you have a doubt about what something was supposed to mean/do. I should be able to tell from the old code.

Alright. I'll try to work on it tomorrow or later. Thanks. 🙂

Panquesito7 added a commit to Panquesito7/minetest-skyblock that referenced this pull request Aug 1, 2021
Revival of cornernote#73 and cornernote#58.
Co-authored-by: Lymkwi <Lymkwi@users.noreply.github.com>
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