Provide a few global console commands by default #54

tonygiang · 2021-04-07T15:18:05Z

I use the Command Console feature extensively for work projects. Among the commands I have written, I identified a few commands that are universally helpful to the purpose of runtime testing no matter which project I use this package for. It makes sense to me that these universally-helpful commands should be shipped by default. This can be the beginning of more than just help and sysinfo being shipped by default with In-game Debug Console (IGDC). The command groups I created are:

PlayerPrefsCommands: There is actually no readily-available tool to read and modify PlayerPrefs in-game for Unity. So I thought: with very minimal code addition, IGDC could just fill that gap. The commands are now prefs.int, prefs.float, prefs.string, prefs.delete, prefs.clear.

TimeCommands: Just a command to get and set the game's time scale value and to read the current time scale: time.scale. I use this to manipulate time during debug session without creating an extra button on the UI for each scene context.

SceneCommands: 4 commands to load, unload and restart scenes. They are scene.load, scene.loadasync, scene.unload, scene.restart. scene.load and scene.loadasync accept a load mode parameter too.

Because IGDC is particularly useful for mobile test environment where typing is really a chore, I tried to make command names short and parameters as case-insensitive as possible. We can update the documentation to reflect the new commands later, but fortunately, help command already listed all commands by default. The command names are denoted with "namespaces" as you can see. This was symptomatic of our work projects due to short command names, high chance of command naming conflicts, which led to us segmenting the commands into namespaces. Turns out IGDC has no problem accepting . character in command names so we rolled with it. You can amend these names if you don't like the namespaces.

yasirkula

Thank you for the PR! Please take a look at the suggested changes.

yasirkula · 2021-04-09T11:30:17Z

Plugins/IngameDebugConsole/Scripts/Commands/SceneCommands.cs

+		}
+
+		[ConsoleMethod( "scene.unload", "Unload a scene", "sceneName" )]
+		public static void UnloadScene( string sceneName ) =>


Old Unity versions that use C# 4.0 doesn't support this method declaration syntax, it needs to be fixed

Similar to scene.load, this command should have a separate scene.unloadasync variant

A helper scene.restart command would also be nice. It shouldn't take any parameters and simply reload the currently active scene. You can define a separate scene.restartasync variant, if you wish

yasirkula · 2021-04-09T11:33:37Z

Plugins/IngameDebugConsole/Scripts/Commands/SceneCommands.cs

+		{
+			if ( SceneManager.GetSceneByName( sceneName ).IsValid() )
+			{
+				Debug.Log( $"Scene {sceneName} already loaded" );


Old Unity versions that use C# 4.0 doesn't support interpolated strings, it needs to be fixed. I plan to maintain Unity 5.6 support for as long as possible.

yasirkula · 2021-04-09T11:36:53Z

Plugins/IngameDebugConsole/Scripts/Commands/PlayerPrefsCommands.cs

+		[ConsoleMethod( "pp.set",
+			"Set the value of a PlayerPrefs field",
+			"key",
+			"type",
+			"value" )]


Parameter names in ConsoleMethod are optional. Since the entered parameter names are the same as the actual parameter names here, we can simply omit these optional parameter names from ConsoleMethod and convert it to a single-line attribute.

yasirkula · 2021-04-09T11:38:09Z

Plugins/IngameDebugConsole/Scripts/Commands/PlayerPrefsCommands.cs

+{
+	public class PlayerPrefsCommands
+	{
+		[ConsoleMethod( "pp.get",


Instead of pp, prefs would be much easier to understand.

yasirkula · 2021-04-09T11:45:50Z

Plugins/IngameDebugConsole/Scripts/Commands/SceneCommands.cs

+	public class SceneCommands
+	{
+		[ConsoleMethod( "scene.load", "Load a scene", "sceneName", "mode" )]
+		public static void LoadScene( string sceneName, string mode )


I see that you've defined mode's type as string so that it can be parsed case-insensitively. I'll update the internal enum parse code so that it will support case insensitive enums. So please change mode's type to LoadSceneMode

This function should be split into 4 different functions: 2 of them use LoadScene and the other 2 use LoadSceneAsync (command name can be "scene.loadasync"). And 2 of these functions should take LoadSceneMode mode parameter while the rest should only take string sceneName parameter

yasirkula · 2021-04-09T11:47:26Z

Plugins/IngameDebugConsole/Scripts/Commands/TimeCommands.cs

+		[ConsoleMethod( "time.scale", "Set the Time.timeScale value", "value" )]
+		public static void SetTimeScale( float value ) => Time.timeScale = value;
+
+		[ConsoleMethod( "time.currentscale", "Get the current Time.timeScale value" )]
+		public static float GetCurrentScale() => Time.timeScale;


These can both be time.scale. The console already supports console method overloads

While assigning to timeScale, value must be Mathf.Max'ed to make sure that it is never negative

yasirkula · 2021-04-09T11:58:07Z

Plugins/IngameDebugConsole/Scripts/Commands/PlayerPrefsCommands.cs

+			"Get the value of a PlayerPrefs field",
+			"key",
+			"type" )]
+		public static string PlayerPrefsGet( string key, string type )


I don't like the "string type" approach. How about this:

prefs.int: is connected PlayerPrefsGetInt and PlayerPrefsSetInt functions. So if user types prefs.int key, it returns that key's int pref. If user types prefs.int key 1, it sets that key's int pref to 1.

Similarly, prefs.float and prefs.string

yasirkula · 2021-04-09T12:05:23Z

Plugins/IngameDebugConsole/Scripts/Commands/PlayerPrefsCommands.cs

+			if ( type.ToLower() == "int" ) PlayerPrefs.SetInt( key, int.Parse( value ) );
+		}
+
+		[ConsoleMethod( "pp.del", "Delete a PlayerPrefs field", "key" )]


prefs.delete would be more preferable

yasirkula · 2021-04-09T12:07:53Z

Plugins/IngameDebugConsole/Scripts/Commands/PlayerPrefsCommands.cs

+	public class PlayerPrefsCommands
+	{
+		[ConsoleMethod( "pp.get",
+			"Get the value of a PlayerPrefs field",


Please use present tense to be consistent with help and sysinfo. Instead of Get, prefer Returns

…4 compatible, omit optional parameter names in command attributes, add scene.restart command.

tonygiang · 2021-04-09T13:23:18Z

I didn't know about command overloading, good to know. I'm incorporating your requested changes, but I do have one concern about picking async/sync mode for scene unload: SceneManager.UnloadScene has been obsoleted by Unity with the following warning message: Use SceneManager.UnloadSceneAsync. This function is not safe to use during triggers and under other circumstances. See Scripting reference for more details.

Are you sure you still want a sync mode for scene unload moving forward?

yasirkula · 2021-04-09T13:25:45Z

Didn't know about that. We don't need UnloadScene in that case.

tonygiang · 2021-04-09T13:41:25Z

The requested changes are live for review.

yasirkula · 2021-04-10T14:43:18Z

Thank you for the changes!

…s better to have these commands opt-in rather than opt-out. Built-in commands can be reactivated by adding IDB_ENABLE_HELPER_COMMANDS compiler directive in Player Settings

tonygiang added 7 commits April 7, 2021 18:50

Add PlayerPrefs commands.

0a158a8

Remove unused parameter for PlayerPrefsClear.

7b90b11

Add TimeCommands.

1a7372d

Add SceneCommands.

3f99c50

Rename PlayerPrefsCommand file name.

3876256

Reformat new files to follow contribution guidelines.

7883c1d

More formatting to follow guidelines.

691f757

yasirkula requested changes Apr 9, 2021

View reviewed changes

tonygiang added 2 commits April 9, 2021 20:14

Many fixes, add async/sync option to scene commands, make commands C#…

4e4d1bb

…4 compatible, omit optional parameter names in command attributes, add scene.restart command.

Pass in load mode for scene commands by enum.

a0033ff

Various changes to reorganize commands.

97b4f9d

Fix the formatting of PlayerPrefsDelete

7816875

yasirkula merged commit dda9bd3 into yasirkula:master Apr 10, 2021

Uh oh!

Provide a few global console commands by default #54

Provide a few global console commands by default #54

Uh oh!

Conversation

tonygiang commented Apr 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yasirkula left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonygiang commented Apr 9, 2021

Uh oh!

yasirkula commented Apr 9, 2021

Uh oh!

tonygiang commented Apr 9, 2021

Uh oh!

yasirkula commented Apr 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tonygiang commented Apr 7, 2021 •

edited

Loading