Cmdr icon indicating copy to clipboard operation
Cmdr copied to clipboard

Filesystem-like view of the game tree

Open evaera opened this issue 5 years ago • 21 comments

Optional command group consisting of commands for navigation and modification of the game tree.

evaera@DataModel cd Workspace evaera@Workspace ls evaera@Workspace prop Baseplate CanCollide false evaera@Workspace rm Baseplate

Potential commands cd, pwd, rm, cp, mv, make <instance> [name], prop <name> <property> [value to set]

prop's value parameter could use dynamic Type from #93

evaera avatar May 07 '19 06:05 evaera

I would definitely love to see this as a feature. Not only does it give some major bash/linux vibes, but it's really useful for debugging.

NobleDraconian avatar May 27 '19 16:05 NobleDraconian

Working on this!

One edge case I would like to point out so far regarding relative paths, is that "game" is a reserved keyword. So if there is a part named "game" under game.Workspace, doing the following with relative paths won't work as expected:

cd Workspace
cd game

This will change the 'working directory' back to game, instead of game.Workspace.game.

If an absolute path is specified, it will work as expected. i.e.

cd game.Workspace.game

This will change the 'working directory' to game.Workspace.game.

This applies to any of the 'linux commands' as it were.

NobleDraconian avatar Oct 18 '19 05:10 NobleDraconian

@Reshiram110 support cd .game

LoganDark avatar Oct 18 '19 06:10 LoganDark

support cd .game

@LoganDark Could you clarify a bit more on this? Are you asking me to reserve ".game" as game, instead of "game"? I.e. to switch to game you would run

cd Workspace
cd .game

NobleDraconian avatar Oct 18 '19 06:10 NobleDraconian

cd game -> cd to datamodel cd .game -> cd to the instance named "game" in the 'working directory'

That would allow you to reserve other keywords without making it any harder to navigate

You could have game, workspace, character (your character model) or char, etc.

LoganDark avatar Oct 18 '19 06:10 LoganDark

With this change, I would like to rename the SetPlaceName function to SetInputLabel. The old SetPlaceName should be assigned to the new function so they both work.

evaera avatar Oct 24 '19 11:10 evaera

@evaera Is this what you were looking for? https://github.com/Reshiram110/Cmdr/commit/61307d03408accd724fd6e81514aa5254b580974

NobleDraconian avatar Oct 29 '19 02:10 NobleDraconian

@evaera Why not SetPrompt? :<

LoganDark avatar Oct 29 '19 02:10 LoganDark

Personally, I find SetPrompt to be more clear and concise.

NobleDraconian avatar Oct 29 '19 02:10 NobleDraconian

SetPrompt is better, let's use that

evaera avatar Oct 29 '19 02:10 evaera

The treeview now supports ls'ing/cd'ing on instances who's names are the same as a keyword's!

I.e. if you have a part under Workspace named "game", you can cd to it/ls it via

cd Workspace
cd .game

This is the same as doing cd ./SomeFolder in bash. It should be noted that "game" is a reserved keyword, so if you ran "cd game", the current working instance would be changed to the datamodel's root.

cd Workspace
cd game

NobleDraconian avatar Oct 29 '19 02:10 NobleDraconian

Just found an edge-case, if you cd into an instance (say Workspace.Part for example), then the instance gets deleted, any attempt to ls or cd with any relative path to the now deleted instance fails, and instead simply returns the standard "{Name} is not recognized as a valid instance".

I ran a quick experiment in linux bash, and it suffers from a similar problem: image

This brings up the question of how we should handle cases where the current working instance has been deleted. Should we just cd back to game? I toyed with the idea of cding to the deleted instance's parent, but that isn't really feasible as the instance has been deleted..meaning its parent property no longer exists.

EDIT : Perhaps instead of just storing the current working instance as an actual instance, I could just store the string path to it and resolve it when needed? This allows us to use simple string manipulation to get to the first valid parent instance in the event the current working instance is deleted.

cd Workspace.Part.SubPart rm game.Workspace.Part From this point, we are now currently cd'd into a deleted instance. We can simply check the instance path string and resolve each instance until we hit a valid, existing instance and cd to there autmomatically. game.Workspace.Part.SubPart will be changed to game.Workspace, since Workspace is the first valid and existing instance.

NobleDraconian avatar Nov 16 '19 05:11 NobleDraconian

So after some discussion with @evaera, it seems the best course to take here is to just handle the instance in a detached state until the developer cd's back into the datamodel.

NobleDraconian avatar Nov 16 '19 07:11 NobleDraconian

Instances detached from the datamodel are now supported! If the currently active working instance is destroyed, a warning will be displayed in the terminal.

Here's a quick rough example of how the behavior looks:

[email protected]:game.Workspace.Reshiram110.Head$ ls
Mesh
Neck
etc.

I die, avatar is thus destroyed and parented to nil

The current working instance has been destroyed, unexpected behavior may occur!

[email protected]:Reshiram110.Head$ ls
The current working instance has been destroyed, unexpected behavior may occur!

Mesh
Neck
etc.

A few seconds later, the garbage collector destroys the rest of the avatar except the head

[email protected]:Head$ cd Mesh
The current working instance has been destroyed, unexpected behavior may occur!

[email protected]:Head.Mesh$

Garbage collector destroys head

[email protected]:Mesh$

NobleDraconian avatar Nov 16 '19 10:11 NobleDraconian

@Reshiram110 unexpectedbehavior 3 times, maybe you should fix that :)

I have a little proposal, if the current instance's parent is nil then don't allow cd .. (or cd ... to go up 2 directories and so on... you support that right? is useful!!!)

Additionally cd ".." to go into an instance literally called .. would be useful too, but I don't know if Cmdr would allow it

LoganDark avatar Nov 16 '19 11:11 LoganDark

@LoganDark

unexpectedbehavior 3 times, maybe you should fix that :)

I could limit it to display once, during the initial time the working instance has been destroyed instead of every time you cd or ls under a destroyed instance.

if the current instance's parent is nil then don't allow cd ..

I don't see any benefit to dis-allowing commands to be ran while under a destroyed instance - if the developer is working under a destroyed instance, they may be doing it for debugging purposes or somesuch and thus limiting the behavior would only hinder them. Limiting the behavior would also force me to remove the generalized behavior of the code that resolves relative paths. The only 'benefit' I see to doing this is to suppress the excessive warning messages, which, as stated in my first response, can be easily changed.

or cd ... to go up 2 directories and so on... you support that right? is useful!!!

This currently is not supported, .. is used to traverse up one instance in a relative path. I'm not entirerly sure how I would implement something like this due to the structure of Roblox instance paths. Also, the syntax feels a bit weird to me - cd .... to go up 3 instances doesn't seem very ergonomic as there's nothing separating each individual .. In bash/windows you have cd ../../../../, but since we're wrapping around the structure of instances, we can't natively add a 'path separator', and not having one makes it hard to read where exactly you are navigating to.

NobleDraconian avatar Nov 16 '19 17:11 NobleDraconian

I could limit it to display once, during the initial time the working instance has been destroyed instead of every time you cd or ls under a destroyed instance.

I meant fix the typo! unexpectedbehavior is not a word. You pasted that in 3 times in your comment, didn't notice it, then I pointed it out, and you still didn't notice it? xD

I don't see any benefit to dis-allowing commands to be ran while under a destroyed instance

I never said that, I said cd .., as in... don't allow the user to change their directory to nil.

LoganDark avatar Nov 16 '19 17:11 LoganDark

I meant fix the typo! unexpectedbehavior is not a word. You pasted that in 3 times in your comment, didn't notice it, then I pointed it out, and you still didn't notice it? xD

Oh haha! I actually did notice it a few hours ago, but I was tired and was heading to bed so I didn't bother fixing it. I'll edit my post to correct the typo. :-)

I never said that, I said cd .., as in... don't allow the user to change their directory to nil.

Ah ok. My code already handles this. Since .. gets resolved to Instance.Parent in a relative path, if you cd .. while under game or the root of a detached instance, ls/cd will simply spit '..' is not recognized as a valid instance to the terminal, since .. is nil in that case.

Example:

[email protected]:game$ cd ..
'..' is not recognized as a valid instance.
[email protected]:game$

NobleDraconian avatar Nov 16 '19 17:11 NobleDraconian

I'll edit my post to correct the typo. :-)

You already did, before I even made my post. You should edit the code to correct the typo too. :P

LoganDark avatar Nov 16 '19 17:11 LoganDark

My code never had that typo, I typed the output examples by hand.

NobleDraconian avatar Nov 16 '19 17:11 NobleDraconian

~So you made the same typo 3 times?~ Oh, okay, got it :p

LoganDark avatar Nov 16 '19 17:11 LoganDark