Make canvas get feature parity with normal drawing functions

[Trivial-Man]

I don't know much of anything about C# or Git or how to try and get things added to or changed for projects like this so bear with me.

In EmuLuaLibrary.Gui there's a command called DrawImage that allows lua scripts to display an image over the normal emulator display. I wanted to be able to do that in a lua canvas, but currently there's no way to do so. Copy/pasting the code from EmuLuaLibrary.Gui to the end of LuaCanvas with some minor alterations allowed me to achieve the same behavior as the original function but in a canvas. Here's the code that I ended up with:

		[LuaMethodAttributes(
			"drawImage", "draws an image file from the given path at the given coordinate. width and height are optional. If specified, it will resize the image accordingly")]
		public void DrawImage(string path, int x, int y, int? width = null, int? height = null)
		{
			if (!File.Exists(path))
			{
				// Some kind of output stating the file wasn't found should go here, but I don't know how to do that
				return;
			}

			Image img = Image.FromFile(path);
			_graphics.DrawImage(img, x, y, width ?? img.Width, height ?? img.Height);
		}

If someone could add the missing output or error display or something to this and then add this to BizHawk I'd really appreciate it.

[zeromus]

This should really be changed to "make canvas get feature parity with normal drawing functions"

[Trivial-Man]

I tried to add this and the other missing drawing features in #882.

[Trivial-Man]

@TASeditor I was told to work this out with you here.
You commented that you can't get images to draw to the canvas still? I'd like to hear more about that since I was able to do so. I expect that the filename is slightly off and that's preventing it from drawing. Unfortunately when that fails it fails silently and I don't know how to fix it.

To be honest, all my code was based on the code already present in EmuLuaLibrary.Gui. I just trimmed out the bits related to copying the emulator's display as the background for what is drawn (I think. I'm still figuring this out). I figured this was a safe way to make sure that the features definitely worked.

[c7fab]

I'm not sure what's specifically broken with this script: https://github.com/TASeditor/asdf Sometimes it works out of nowhere.
I tried many others, including yours, they all worked.

[zeromus]

Like he said, it's probably got to do with the pathing. use process monitor to see what path its trying to use.

[Trivial-Man]

So, for some reason when I open that script up in Eclipse (which I've been using to work on my lua scripts) there's some garbled characters in the file name. Specifically, it reads that the file you are trying to load is "‪test.png". When I deleted the garbage characters everything worked just fine.

[zeromus]

Bizarrely the URL he posted is broken too.

[Trivial-Man]

Yeah, I had to just copy/paste what he typed and ignore the hyperlink.
Also, I tried opening that file in notepad, notepad++, and Visual Studio and none of those garbage characters show up. And obviously they don't show up in github either. Eclipse is the only program I can make display that mess. So I'm not sure what exactly is going on there, but presumably it is some kind of text encoding issue.

[zeromus]

It's E2 80 AA

Beats me.

[Trivial-Man]

Google says E2 80 AA is the UTF-8 character for left-to-right embedding. Trying to use Unicode in any lua script in Bizhawk will give an error and terminate the script. So yeah, this issue is not with the DrawImage function.

---

After these merges there are 3 features in gui that seem completely inapplicable to canvas (DrawNew, DrawFinish, addmessage), and several that seem unnecessary or inappropriate for use outside the emulator display (pixelText and "text" and the associated functions of each). DrawString and defaultTextBackground are the remaining functions implemented in gui and not in canvas.

• DrawString is just an alias for DrawText and would be trivial to implement, but also a bit pointless in my opinion.

• defaultTextBackground would be more complicated because the way drawText is implemented differs between canvas and gui. Still probably not too hard if someone wanted to implement it, but I didn't because I wanted to make sure my changes didn't accidentally break anything that was already there.

Having said that, everything I wanted is implemented now. Someone more knowledgeable than me should eventually figure out how to make an error message of some sort print when a function fails, but really the fact that I was able to get anything here to do what I wanted surprised me. Thanks for bearing with me @zeromus.

[c7fab]

The encoding issue would explain why sometimes I had trouble with my scripts in general in the past, but still weird they sometimes worked out of nowhere.
I added the stuff from Trivial-Mans list in PR: #883

Stuff to implement:

• A function to get mouse and keyboard buttons/position
• A function to set the postion, maybe in the constructor too

[Trivial-Man]

You can already get mouse and keyboard information using the input.get and input.getmouse functions. Why would these be duplicated in the canvas code?

[c7fab]

It doesn't work when canvas window has focus, maybe you had background input enabled. And it would mess up coordinates, because it takes the client ones.

[Trivial-Man]

In #885 I implement the error messages for when image files are not found (among other more important changes). Coupled with @TASeditor's changes I believe this would bring LuaCanvas up to feature parity with the gui drawing functions.
Technically pixelText and "text" aren't supported, but as I mentioned earlier I don't think they are useful outside the game display, so I'd have to hear someone actually request them to have any desire to implement them.

[zeromus]

So its only you two guys who know whats going on. Is it all in #885? @TASeditor, please sign off

[c7fab]

@zeromus Not everything from this conversation is in #885.
Missing: "A function to get mouse and keyboard buttons/position" and "A function to set the postion, maybe in the constructor too". I'm not sure wheter @Trivial-Man wants to add them.

[Trivial-Man]

I'm happy to look into adding them. I have not done so yet so no promises on whether or not I can, but I agree they would be useful. The only reason I hadn't done so yet was because I was waiting on any sort of feedback on this somewhat major change already pending.

[zeromus]

do it. if all youre doing is making luacanvas not suck so much, there should be nothing contentious besides what you two argue about

[Trivial-Man]

Will do. By Thursday I should have something ready.

[Trivial-Man]

I'm working on including a function that returns mouse coordinates relative to the canvas window, but I do not think I will be able to add functions that read inputs like button presses and mouse clicks. While TASeditor is correct that input.get doesn't return anything unless the emulator window has focus, input.getmouse does still return whether the mouse buttons are clicked (though the scroll wheel doesn't work without focus).

Given that LuaCanvas is supposed to be a simple display window, I think adding interactive functionality like getting button presses and such is beyond its scope. The reason I created LuaPictureBox and made it a component that could be added to forms was so that displays could be used in user created windows that also have interactive elements (buttons, text boxes, drop down lists, ect.). And besides, as long as the emulator window has focus input.get will still work and can be used to trigger changes on the canvas.

If button/keyboard presses are actually something that people want to use when the emulator window doesn't have focus, I suggest rewriting the input functions that already exist to accept background input (maybe optionally depending on a parameter) rather than trying to replicate this functionality.

Any thoughts on this @TASeditor (and to a lesser extent @zeromus)?

EDIT:
Incidentally, I ran into #867 while testing the input functions. I don't have a clue what to do about that, so that also might be a good incentive for not adding more places where scripts can crash.

[zeromus]

It's not an incentive for anything but fixing lua.

I dont care whether the input works or not i just want you two to collaborate on it until youre done collaborating.

[Trivial-Man]

@TASeditor The newest commits to my PR add functions to get the mouse position and set the canvas window's location. I've tested everything a bit and it seems to be behaving as I expect, but if you find any bugs let me know.

Assuming that what's already in place works for you, I believe I am done adding features. You're welcome to present a case for getting button inputs when the emulator window doesn't have focus, but unless it is very compelling that's something I'd rather leave to you or a future contributor.

[c7fab]

Given that LuaCanvas is supposed to be a simple display window, I think adding interactive functionality like getting button presses and such is beyond its scope.

Dragging the view of the canvas would require mouse presses or clicking into the object displayed. And besides that the emulator window never has focus for me.
And what if an user wants to have different function call for clicking on canvas than for clicking on emulator window?

Your latest commit works fine.

[Trivial-Man]

Admittedly, I had assumed that the addclick function from the forms would Just Work™ with any of the components added to it. I hadn't bothered to test it until now. Seems I was wrong! It only works with buttons as far as I can tell.

My whole argument before this was based on the idea that the forms class could handle events like clicks and key presses automatically and that they didn't need to be programmed into each component class. I was totally wrong though. I'm going to add a quick commit to this PR later tonight soon to add the simple event that the canvas/PictureBox component is clicked, but after that I think I'll open a separate PR for adding more robust support for mouse and keyboard input in canvases and forms. After this last commit the original issue will be solved and then some, and I'd rather get what's here merged before delaying with another big set of only tangentially related changes.

[Trivial-Man]

Sorry for how long this took me. Work really messed me up lately.

Adding the onClick event to LuaPictureBox components added to a form is pretty easy. Unfortunately adding it to a LuaCanvas is much harder. I'd either need to add some sort of boolean value to the LuaPictureBox and have the LuaCanvas set it when it's called and then change some behavior later on depending on what that value is, or I'd need to find some way of overriding the onClick method for LuaPictureBox from within LuaCanvas.

Either way, even if I could get clicks being registered by LuaCanvas, I'd then have to do some shenanigans to get the canvas to actually store and be able to recall user defined Lua functions. It'd take a lot of duplicating code in LuaWinforms or some sort of rewriting so that the canvas itself already is a LuaWinform.

All of that is a bit much for me. I'm content with only adding this functionality to Forms. If TASeditor or someone else wants to pursue this later, they are welcome to, but I have other things I want to work on. Once my PR is merged I'm closing this.

[Trivial-Man]

Feature parity has been more than achieved. Requests for other features for the canvas should be made in another issue.