Additional Tests

[halotroop2288]

Test for: Homebrodot/Godot#10 and Homebrodot/Godot#14

[hokaze]

Having a combined test suite project is a good idea.

I think ideally it could do with having a main scene as a menu that lets you pick which test to run then loads that scene - as otherwise it just does battery test by default and you'd have to export 2 nros to test both, needing to manually change the main scene in the editor to the keyboard scene + export again to test the latter.

Will do a proper review later, once I've built a version of the editor that includes the relevant PRs so the keyboard scene actually works and doesn't complain about the NintendoSwitch singleton being undeclared, and can confirm it all works on my end too

[halotroop2288]

OK for now they both fit in one scene, so I just added them both to one scene.

[hokaze]

Went into more detail in comment on BatteryTest.tscn, but the two issues specific to the test suite are:

• BatteryTest _update_labels has a typo, script line 39 in the godot editor, should be battery_percent_label and _battery_percent instead of battery_seconds_label and _battery_seconds

• GridContainer for 2 scenes ends up squishing them together and cuts off the TextEdit node from the SoftwareKeyboardTest.tscn entirely, need to resize, set minimum sizes or drop the container, so both scenes are fully visible

Misc stuff that came up from testing:

• battery stuff works! haven't tested all possible states, but reports Charging/Discharging correctly, and battery percent when the typo is fixed
• keyboard stuff doesn't work, neither of the 2 buttons nor focusing on the textedit bring up any keyboards, not had a proper look at that PR to comment on why, I'm afraid

[hokaze]

I'm happy with the changes to the test suite and have ran it (again) on hardware.

I've not tested it with a template built from your most recent commits to the virtual keyboard API, but that PR aside the test suite seems fine for testing the power + keyboard PRs.

Only other comment I have is that I think the script tied to the SofwareKeyboardTest scene where it shifts the position if the keyboard is open might be slightly wrong (it's hard to tell without the keyboard actually visibly opening):

func _process(delta: float):
	self.rect_position.y = original_position.y + OS.get_virtual_keyboard_height()

Shouldn't that be a minus, not a plus, in order to shift the textbox up so it's above the inline keyboard? Currently it moves it down instead, and at least based on where the previous inline keyboard appeared, that would make the text be hidden behind the keyboard when it'd actually still be visible if not moved.

Unless you're expecting the keyboard to come in from above instead of from below, or I've missed something. Either way, can confirm the scene shifts when you use either of the buttons for opening the keyboard or focus on the text edit (even if the actual keyboard doesn't visibly appear) and then shifts back into position when unfocused.

[halotroop2288]

Shouldn't that be a minus, not a plus, in order to shift the textbox up so it's above the inline keyboard?

Yes, but now I'm thinking this is wrong anyway. It should be adjusting the viewport to exactly where it should be, and it's a bad assumption that it would always need to be exactly that much higher, as the textedit might not be in the center of the screen, especially when we start adding more things to the main scene.

[hokaze]

Well, you've certainly been busy!

This all looks solid to me, good ideas to add to the suite and the 3D camera so we can test rendering at the same time.

(the hidden credits section is also a nice touch)

I've noticed the better handling for padding when the keyboard is visible and am fairly confident that should work as intended when the number/language keyboard variants are working

It's a shame GDScript has no equivalent to #ifdef that I'm aware of, as ideally I think the software keyboard test should have some way of only calling the "NintendoSwitch.show_virtual_keyboard( ... )" code if that singleton is defined in the engine, as currently we can't actually use the keyboard test scene with the main branch (even just to check the default keyboard against the text edit), as it needs the Switch Virtual Keyboard API PR code in order to run and the scene doesn't load due NintendoSwitch being undefined.

[halotroop2288]

It's a shame GDScript has no equivalent to #ifdef that I'm aware of, as ideally I think the software keyboard test should have some way of only calling the "NintendoSwitch.show_virtual_keyboard( ... )" code if that singleton is defined in the engine

It does have something similar. OS.get_name() == "Switch" would work.

[hokaze]

It does have something similar. OS.get_name() == "Switch" would work.

I mean, that'll work once the [3.5 | Switch] Create Switch API for virtual keyboards PR is merged, but isn't actually neccessary or even an issue - running on Windows/Linux you can access the software keyboard scene because the NintendoSwitch singleton is still defined in the editor (if built with that PR's code present), so it doesn't complain.

It's if you use the current Homebrodot editor or initial 3.5.3 release that any executable of the test suite runs into that issue, I find, regardless of what platform it's running on.

Not worth holding the test suite back over, as realistically we're going to be testing the software keyboard functions with that PR's code, more an absent musing that it'd be nice whenever adding new stuff not in base Godot to be able to still have those scenes function.

EDIT - interestingly, it looks like if the editor is built with the keyboard PR but the template is built without the keyboard PR, it loads the scene fine, albeit with some errors logged over nxlink, at which point the current main brain keyboard works fine. Weird hack, but ended up being useful for reviewing the keyboard controller input fix in the [Switch] Implement OS Functions PR