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

[BUG - Release] (LUA) RegisterConsoleCommand_ - Console Ignoring Strings #653

Closed
alphaqueueop opened this issue Sep 13, 2024 · 16 comments · Fixed by #669
Closed

[BUG - Release] (LUA) RegisterConsoleCommand_ - Console Ignoring Strings #653

alphaqueueop opened this issue Sep 13, 2024 · 16 comments · Fixed by #669

Comments

@alphaqueueop
Copy link

Branch or Release
v3.0.1
d935b5b

Game and Engine Version
probably any

Describe the bug
console does not recognize strings and treats all spaces as a parameter

Mods directory
...

To Reproduce

function modify(obj,prop,val)
    item = FindFirstOf(obj)
    if item then
        item:SetPropertyValue(prop,val)
    end
end

RegisterConsoleCommandHandler("modify", function(FullCommand, Parameters)
    if #Parameters < 1 then return false end

    obj = Parameters[1]
    prop = Parameters[2]
    val = Parameters[3]
    modify(obj, prop, val)

    return true
end)

Expected behavior
strings to be processed properly
"foo bar"
'foo bar'
['foo bar']
none work

spaces and command both act as seperators no matter what

Screenshots, UE4SS Log, and .dmp file
input
>modify Ultra_Dynamic_Sky_C 'Time of Day' 0
output
[Lua] of

Desktop (please complete the following information):
Win10

Additional context...

@igromanru
Copy link
Contributor

Need feedback from other UE4SS Devs.
If I would want to implement the feature, should it only work for double quotes or single quotes as well?

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 28, 2024

Need feedback from other UE4SS Devs. If I would want to implement the feature, should it only work for double quotes or single quotes as well?

I know UE refers to objects with paths sometimes with single quotes, so those might need to go in double quotes if passed as a console command param, therefore should we only accept double quotes ?
I don't remember exactly when UE uses single quotes for objects, but I've definitely seen it somewhere.

@igromanru
Copy link
Contributor

Indeed, I think in some rare cases single quotes can be in full name paths as well.
I'm planning to handle escape characters, like allow users to write \" to get " out of it.
But I think double quotes should be enough.

@narknon
Copy link
Collaborator

narknon commented Sep 28, 2024

It uses single quotes for certain types of paths.
E.g.,
MyBlueprint_C '/Game/Stuff/MyBlueprint.MyBlueprint_C'

@igromanru
Copy link
Contributor

I've noticed that all current console related hooks/handlers return the full command string as first callback parameter.
Is this behavior intentional, or should I fix it as well?
Currently the mod dev has to extract the command name by himself from the whole string.

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 28, 2024

I've noticed that all current console related hooks/handlers return the full command string as first callback parameter. Is this behavior intentional, or should I fix it as well? Currently the mod dev has to extract the command name by himself from the whole string.

The dev docs describe the correct behavior for all three different ways of dealing with commands.
If they behave differently, that's a bug.
https://docs.ue4ss.com/dev/lua-api/global-functions/registerconsolecommandhandler.html
https://docs.ue4ss.com/dev/lua-api/global-functions/registerconsolecommandglobalhandler.html
https://docs.ue4ss.com/dev/lua-api/global-functions/registerprocessconsoleexecposthook.html

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 28, 2024

The examples are wrong in the docs, that's where the confusion is coming from I think.

@igromanru
Copy link
Contributor

igromanru commented Sep 28, 2024

I'm not sure what "command" means.
In RegisterConsoleCommandGlobalHandler and RegisterConsoleCommandHandler it says "The name of the custom command", but under Examples it says FullCommand and returns the full string as example output.
Similar in RegisterProcessConsoleExecPostHook and RegisterProcessConsoleExecPreHook, it just says "The command".

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 28, 2024

Actually, I'm wrong in my previous comment.
The implementation and the docs examples are correct.
The docs are wrong when they say the first param is "The name of the custom command".
The first command being the full command is intentional to allow any kind of custom parsing in case what we supply for the params table isn't good enough for the mod creators purpose.

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 28, 2024

Actually, I'm wrong in my previous comment. The implementation and the docs examples are correct. The docs are wrong when they say the first param is "The name of the custom command". The first command being the full command is intentional to allow any kind of custom parsing in case what we supply for the params table isn't good enough for the mod creators purpose.

Same for the ProcessConsoleExec hook, it's intended to be the full command as one param, and then a table of the param contents as another param for convenience.

I believe this example is wrong where it says full command is in param[0]:

-- Entered into console: CommandExample 1 2 3
-- Output
--[[
Custom command callback for 'CommandExample' command executed.
Full command: CommandExample 1 2 3
Number of parameters: 3
Parameter #1 -> '1'
Parameter #2 -> '2'
Parameter #3 -> '3'
Parameter #0 -> 'CommandExample'
--]]

@igromanru
Copy link
Contributor

igromanru commented Sep 28, 2024

I find it kind of weird that ProcessConsoleExec hook gives only the "paramters". If the first callback parameter should be the full command, so user can parse it, Then the CommandsParts argument in the Callback should contain the "command name" as well, otherwise the user HAS to parse the command anyway just to get the command name.
While it makes sense in register handler functions that you get only parameters, in ProcessConsoleExec hooks it doesn't, since you don't know which command was actually called.

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 28, 2024

I find it kind of weird that ProcessConsoleExec hook gives only the "paramters". If the first callback parameter should be the full command, so user can parse it, Then the CommandsParts argument in the Callback should contain the "command name" as well, otherwise the user HAS to parse the command anyway just to get the command name. While it makes sense in register handler functions that you get only parameters, in ProcessConsoleExec hooks it doesn't, since you don't know which command was actually called.

Yes this is true, we should definitely include the name in CommandParts.

@UE4SS
Copy link
Collaborator

UE4SS commented Sep 28, 2024

I find it kind of weird that ProcessConsoleExec hook gives only the "paramters". If the first callback parameter should be the full command, so user can parse it, Then the CommandsParts argument in the Callback should contain the "command name" as well, otherwise the user HAS to parse the command anyway just to get the command name. While it makes sense in register handler functions that you get only parameters, in ProcessConsoleExec hooks it doesn't, since you don't know which command was actually called.

Yes this is true, we should definitely include the name in CommandParts.

The only question is, do we break Lua common usage and make it CommandParts[0] to not break any potential mods that rely on CommandParts[1] being the first param to the command ?

@igromanru
Copy link
Contributor

Damn, both not ideal. But I find the second solution worse.

UE4SS pushed a commit that referenced this issue Oct 10, 2024
)

* feat: Implement explode_by_occurrence_with_quotes
feat: Replace explode_by_occurrence with explode_by_occurrence_with_quotes in all ProcessConsoleExec functions

* feat: In RegisterProcessConsoleExecPreHook and RegisterProcessConsoleExecPostHook the callback parameter CommandParts should contain all separated parts of the command, the "command name" as well

* docs: Fix and Improve description and examples for RegisterConsoleCommandGlobalHandler, RegisterConsoleCommandHandler, RegisterProcessConsoleExecPreHook and RegisterProcessConsoleExecPostHook

* docs: Add function documentation to explode_by_occurrence_with_quotes
@alphaqueueop
Copy link
Author

Wrapped with "quotes" addition with insensitive casing works wonders and congratulations on the fix! String-spaced values seems to be fixed for now until UE allows more odd input.

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 a pull request may close this issue.

4 participants