Input class refactor

Hello! I’ve started to change some stuff in the iron Input class and i would like suggestions on that. Please see how it is going here https://github.com/knowledgenude/input-class-refactor

NewVI is the new VirtualInput class
NewKB is the new Keyboard

  • Currently both Keyboard and Mouse classes are working fine, but this variables must be changed to functions to work: mouse, movementX, movementY

  • Keyboard class isn’t handling repetitions anymore, this could affect ArmorPaint for example. Maybe we can create a custom class that handles this or a simple function.

We can look for a workaround on that to keep compatibilty later. My plan is to extend this classes with the input map, but it is not a priority now.

3 Likes

Very nice, pretty clean and understandable code!

Without testing it, two points that came into my mind when scanning over the code:

  1. The big if/else-if block at the end of the file can be replaced with a switch. Otherwise (if the compiler isn’t clever enough to optimize this) every single if before the last one will have to be checked before returning F12 for example.

  2. The implementations of NewVI.started() and NewVI.released() have a side effect of which I’m not sure if it’s actually wanted or not: if you call one of those functions twice on the same frame, only the first one will return true because it will remove the key from the corresponding array. I think that usually one would expect released() to be true if the key was released on the last frame. Is that what you meant with repetitions above?

2 Likes

Thanks for the suggestions!

That ifs will be removed anyway i just moved it to another class to avoid the need to scroll that :slight_smile:

But i guess this compiler condition must stay out that switch statement, right?

	else if (key == KeyCode.Shift) return "shift";
	else if (key == KeyCode.Control) return "control";
	#if kha_darwin
	else if (key == KeyCode.Meta) return "control";
	#end

Yes, the key can be pressed only once and will be removed from started/released keys list. Previously if i understood the code, there was a function reset() that is called sometime to clean that keys list.

One advantage of the way of removing after calling, is that the started/released functions would not be dependent of update rate (results in these functions can’t be used by the render2D notifier for example or in other situations that idk). But if you never call the started method the key would not be removed from that list :confused:

So maybe in the end the better way would be to reset that lists in the Input class.

2 Likes

I think it should stay there as it probably handles some MacOS specific behaviour that is otherwise not correctly handled by Kha or so. Better don’t remove it if your unsure why it’s used.

Using an update function might not be the “nicest” way possible, but it certainly works and it’s easy to understand, and that’s what matters. Looking forward to the final refactor :slight_smile:

1 Like

I thought that the compiler condition would not work inside the switch expression but it works fine :slight_smile:

Thanks for the suggestions again! I pushed another commit, so take a look when you can if there is no problem.

Another question:

  • The GamepadSticky class have a lot of variables in common with Pen and Mouse, so would be cool to have this classes extending another class, but this was possible just for Pen and Mouse. The PointingDevice class extends the VirtualInput class to handle that variables, but if i create a PointingDevice instance for each gamepad sticky would be a mess even if the compiler removes the unused codes later, right?

I have another question and i hope this don’t sound stupid… The Pen only have a key currently (“tip” key). Then the started/down/released methods should have the default value “tip” for the Pen. But i can’t override this method from VirtualInput because it is inlinned already in that super class… Should i remove the inline keyword and override this methods inlinned in each child class? Because i have almost sure that inline should be used there (is a short function).

Also i don’t have sure the way i’m doing the things will keep compatibility. I couldn’t fully understand why Lubos created a virtualbutton class, but i fear this had a dark reason :fearful: The same goes for repeat() method and that keyframes array… :smiley:

2 Likes

You can use compiler flags everywhere, even before and after statements, like such:

#if (!kha_js) @:structInit #end class MyClass {}

Those conditional statements are (as far as I’m informed) not directly handled directly by the compiler, but instead in a preprocessing stage like in C/C++ for example. The flags are evaluated before compilation and the compiler only sees the resulting code.

Making GamepadStick a subclass of PointingDevice should work, you don’t need to create an instance of the superclass, the subclass is also of the super class type. If you can’t find a good class hierarchy, interfaces often work as well, but in this case I think it’s not necessary and would probably make the code more fragmented again.

Yes, in this case you have to remove the inline keyword as inlining happens during compilation. Because of polymorphism (= the fact that you can use subclass instances in places where an object of a superclass is required), the actual method to call depends on the object and that needs to be resolved dynamically.

Maybe you can find some hints in the Git blame output. If you click on the icon between a commit message and the line you can go back in history to that commit to see all changes prior to that commit.

1 Like