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

Placeholder for X doesn't work in commands #80

Closed
SeruhioX opened this issue Mar 29, 2024 · 8 comments · Fixed by #98
Closed

Placeholder for X doesn't work in commands #80

SeruhioX opened this issue Mar 29, 2024 · 8 comments · Fixed by #98
Labels
bug Something isn't working

Comments

@SeruhioX
Copy link

SeruhioX commented Mar 29, 2024

Hi, I have a problem in DeluxeMenus when using the open command with a specific target as follows (replacing with whatever names).

/dm open menu -p:player1

When doing this command, I see the menu and placeholders parsed correctly with the player I put in the command, but it doesn't work in commands, for example:

left_click_commands:
- '[console] money give %player_name% 500'

Instead of giving the money to the player that I put in the command, I receive it, instead of giving it to the specific player

An alternative or in addition to fixing this, would be to know that the command is being executed on behalf of another player and to be able to put a requirement to not be able to execute the command when doing this.

@BlitzOffline
Copy link
Member

Hello. Can you please share the version of DeluxeMenus you've experienced this issue on? You should be able to find the version using /dm

@SeruhioX
Copy link
Author

SeruhioX commented Apr 5, 2024

imagen

It is a version that was passed by discord to solve a problem with JSON messages, I have tried other versions with the same result @BlitzOffline

Minecraft version -> Paper 1.20.4

@SeruhioX
Copy link
Author

Any updates of this?

@BlitzOffline
Copy link
Member

Hello. I have looked into the issue and have found where the problem is. Now I need to decide how -p: should work.

At the moment, the target player affects how placeholders (everywhere except in actions) are parsed. I am going to make placeholders inside actions be parsed for the target player as well.

What I am not sure about is wether we should make each action execute for the target player as well. This would mean actions such as [chat] and [command] would make the target player send chat messages and execute commands.

Waiting for suggestions.

@BlitzOffline
Copy link
Member

UPDATE: I will make it so only placeholders inside actions are parsed for the target. I have decided this after seeing the documentation for the command. I believe that someone reading this would interpret it as actions being executed for the viewer but placeholders parsed for the target.
image

@BlitzOffline BlitzOffline mentioned this issue May 25, 2024
@BlitzOffline
Copy link
Member

BlitzOffline commented May 25, 2024

@SeruhioX I have opened a PR to fix this issue (#98). If you don't want to wait until that is merged, you can test this build:
DeluxeMenus-1.14.1-DEV-local-build.zip

@BlitzOffline BlitzOffline added the bug Something isn't working label May 25, 2024
@SeruhioX
Copy link
Author

@BlitzOffline thank you!

I didn't know where to put it, but maybe a simple placeholder that tells you if the menu is being opened by a user using the "-p:" argument would be nice, giving you the option to block actions from being performed in this case with some requirement.

I also noticed that when a menu has "register_command" set to true, the "-p:" argument, and others stop working, becoming as simple text for the argument defined in the register_command, I don't know if I'm doing something wrong.

@BlitzOffline
Copy link
Member

For you request, I have added it as an idea to this feature request: #62

For your question:
The -p:<target> argument only works for the admin command /dm open <menu> [viewer] [-p:<target>]. This is not something that works for commands set in open_command. At the same time, that command doesn't support arguments.

I think this whole -p argument might need to be rethinked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants