Add set/getPlayerScriptDebugLevel

[knitz12]

While i was making a new account system (MySQL) i found out i couldn't use /debugscript without creating an account in MTA's database AND adding it to the ACL, that was a problem because when i logged in my system i also had to login in MTA's system using /login to be able to view script errors, that's why i created this pull request, with this i could add a /debug command which would simulate /debugscript.

This PR adds two new functions, setPlayerScriptDebugLevel and getPlayerScriptDebugLevel, I'm fairly new to the MTA's codebase so i don't know if i made any mistake.

[Citizen01]

Thanks for your PR.

I have a question: Why not setPlayerDebuggerVisible and setPlayerDebuggerLevel ? (this needs other feedbacks than mine before renaming anything).

Also we dropped the hungarian notation for new devs (even though I prefer when the code is consistant).

Would be great if we could give root as element so it does for everyone. I don't have any valid use case though but I see SetPlayerDebuggerVisible was ready for it:

mtasa-blue/Server/mods/deathmatch/logic/CStaticFunctionDefinitions.cpp

Lines 3051 to 3058 in 9c8ca2e

	bool CStaticFunctionDefinitions::SetPlayerDebuggerVisible(CElement* pElement, bool bVisible)
	{
	assert(pElement);
	RUN_CHILDREN(SetPlayerDebuggerVisible(*iter, bVisible))
	if (IS_PLAYER(pElement))
	{
	CPlayer* pPlayer = static_cast<CPlayer*>(pElement);

[knitz12]

@Citizen01 I modified the functions to work with the root element, i didn't changed the hungarian notation as i don't know what to replace them with.

About the naming: I used setPlayerDebugView* instead of setPlayerDebugger* because of the existing (clientside) function named setDebugViewActive but i'm open to change it if others think its better 😃.

Thanks for the review.

[qaisjp]

You can also enable debugscript for "Everyone" in the ACL to get around this

[knitz12]

@qaisjp Then everyone would have access to /debugscript, which is something i don't want, Or am i missing something?

@CrosRoad95 I fixed some things you mentioned, i also tried to fix the others whitespace changes but git didn't picked them (i don't know why) but i don't think they are relevant as they don't change anything visually.

[qaisjp]

Oh, now I understand your problem.

How about setPlayerScriptDebugLevel(player, boolean/int). Pass false to hide, and pass an integer for the level.

There should be a getter.

[knitz12]

@qaisjp Wouldn't it be better to pass 0 to setPlayerScriptDebugLevel to hide the debugger? In a similar way the getter (getPlayerScriptDebugLevel?) would return 0 if the player had the debugger closed.

This is the current behaviour of /debugscript.

[qaisjp]

I thought about that but don't want to conflict with 0: custom message in https://wiki.multitheftauto.com/wiki/OutputDebugString

[knitz12]

@qaisjp Ok, i added set/getPlayerScriptDebugLevel, passing false to setPlayerScriptDebugLevel hides the debugger and passing an integer shows it.

[patrikjuvonen]

I believe it is better to match these function names as well.

[knitz12]

@patrikjuvonen Done :)

[patrikjuvonen]

Thanks for the changes. This PR is ready to be merged but I'd wait for #821 so we can make returns right for this from the start.

[botder]

The decision in pull request #821 (Pin down return value on failure) is now settled.

[knitz12]

Done @patrikjuvonen :)

[patrikjuvonen]

Thank you!
Good job. This PR is ready to be merged afrer 1.5.7 is released.

[ccw808]

Sorry about late review, but does function.setPlayerScriptDebugLevel really need to be restricted to Admin ACL by default?

[knitz12]

@ccw808 Well, my thought for doing this was that a malicious resource could make a backdoor command which would allow the attacker to read all the debug messages, gaining knowledge on the internals of the server or observing how a specific system reacts to certain inputs etc depending on how the server uses the logging functions.

With that scenario in mind i put it in the admin ACL, also to conform with /debugscript.

[qaisjp]

No, you're right, 0 is better for closing