yasirkula / UnityIngameDebugConsole

A uGUI based console to see debug messages and execute commands during gameplay in Unity
MIT License
2.05k stars 218 forks source link

Added console width resize, and updated resize icon to reflect the feature #52

Closed EricBatlle closed 3 years ago

EricBatlle commented 3 years ago

Added console width resize, and updated resize icon to reflect the feature.

I don't know if it's something that you want on your plugin, but it's something that I missed on your awesome plugin and maybe somebody else too ^^

yasirkula commented 3 years ago

I could pull this request with a few modifications. As the first revision, could you use tabs instead of spaces in the modified lines? And make sure that Windows line endings are used.

EricBatlle commented 3 years ago

I could pull this request with a few modifications. As the first revision, could you use tabs instead of spaces in the modified lines? And make sure that Windows line endings are used.

I've used tabs instead of spaces on a Windows-Visual Studio so all should be nice...but If you say that I'll doble check it!

yasirkula commented 3 years ago

"Files changed" tab shows that spaces are used. Probably a setting in your VS installment causes spaces to be simulated as tabs (i.e. put 4 spaces when you insert a tab and move over 4 spaces when you shift the cursor left and right).

EricBatlle commented 3 years ago

@yasirkula You were completly right, I've normalized the entire file, it's correct now?

yasirkula commented 3 years ago

Now it says that all lines are modified 😄 I'd rather keep the diff restricted to only the modified lines.

EricBatlle commented 3 years ago

Understandable, give me a few minutes to deal with my VS config please

yasirkula commented 3 years ago

That's what I feared: https://github.com/yasirkula/UnityIngameDebugConsole/blame/d5bda6bfb2c684ce5fd6d3421976a5905aede933/Plugins/IngameDebugConsole/Scripts/DebugLogManager.cs. I don't know if it is possible to revert Blame history to previous versions. I can't accept this PR like this, so I'll ask you to create a new PR. But beforehand, let me add some review notes to your current commit so that you could add them in the next PR. Will let you know when I'm finished.

EricBatlle commented 3 years ago

No problem, sorry for the inconvenience, I'll wait your notes. In addition, if you can check if this commit has the correct format, to be sure that the next PR won't be a mess like this one ^^'

yasirkula commented 3 years ago

Now it uses proper tabs, thanks. Can't tell if it uses Windows line endings but I'd like to believe that it is. It'd also be great if you could follow the same code style that I'm using (check where I put spaces in if conditions and function declarations) . Since this is a short PR, it should be relatively easy.

I've tested the code myself and noticed that it resizes the window from left to right. It is counter-intuitive since you'd expect it to resize from the resize button. Do you have any reasons for the current behaviour?

EricBatlle commented 3 years ago

Now it uses proper tabs, thanks. Can't tell if it uses Windows line endings but I'd like to believe that it is. It'd also be great if you could follow the same code style that I'm using (check where I put spaces in if conditions and function declarations) . Since this is a short PR, it should be relatively easy.

Of course, I can follow those guidelines.

I've tested the code myself and noticed that it resizes the window from left to right. It is counter-intuitive since you'd expect it to resize from the resize button. Do you have any reasons for the current behaviour?

Good point, and no, after thinking it...there is no logical reason to be like that, and as you said is counter-intuitive, I'll change that.

Something else?

yasirkula commented 3 years ago

I've added a couple of review notes. Will be waiting for your new PR 😺