PR Review List

[Bloodknight]

Note that these are not the exhaustive list, but they are some of the initial easier ones to pull/test.

https://github.com/GarageGames/Torque3D/pulls exists for a complete list, so feel free to try any of them, pst that you've reviewed them along with any changes if any are needed.

This is a starting list, the current idea is that these can be posted one weekend, reviewed before the following weekend and pulled merged into the engine if all is fine. Feel free to offer feedback, not just on the reviews, but on this process too.

The list

T: = template used

https://github.com/GarageGames/Torque3D/pull/2299 Sqlite Console refactor

needs testing with an actual sqlite test script

https://github.com/GarageGames/Torque3D/pull/2220 Changes for SFXSound::setPosition(time)

Note: Only tested with OpenAL.

https://github.com/GarageGames/Torque3D/pull/2168 Fixes for compiling without Advanced Lighting

CONFLICTS:- rework and review T: Basegame

https://github.com/GarageGames/Torque3D/pull/2144 Allow correct display of widescreen resolutions on a 4:3 display

Recall seeing many warning about DXGI_MODE_xxx while compiling on linux.

https://github.com/GarageGames/Torque3D/pull/2140 adds a slider for the HDR color shift

T: Full

https://github.com/GarageGames/Torque3D/pull/2076 allows different waypoints

Needs quick test before moving to the NOW list

[Happenstance]

Thanks for the taking the initiative on this, @Bloodknight.

I'll tackle what I can by this weekend:

* Update #2168 (fixes for compiling without Advanced Lighting) to fix the conflicts.

* Convert #2140 (HDR slider) to the new templates. Not sure there's value in leaving this in the old Full template considering that's going away next release?

* Test #2076 (waypoint changes). If this works how I think it does (pops up a builder dialog so you can select a datablock) I'll include some quick example scripts that spawn different waypoint types.

[Bloodknight]

It wasn't just me the other members of the Mystic Order of Veiled Prophets of the Enchanted Realm were also involved. And while the waters were muddied somewhat in the original thread, nothing goes unnoticed, and I have time so I volunteered my services.

I suspect that the whole of the PR review over the near term may well be messy, there's a lot of them, and as you noted, many of them have been there for years, if these cannot be easily merged and tested it is my opinion that they should be moved to issues so they can be re-addressed either by other commiters or the original authors if they are still around.

And yes, I realise that this messes up the issues section, and hopefully, that section will be getting the same treatment, I will, however, state for the record that I'm less than happy with the way 'issues' are handled as a whole by Github, the problem with flexible systems is they are messy systems, i'd prefer a much cleaner interface to distinguish between bugs/issues and feature requests/improvements the latter set imo are not 'issues' except for people who want that feature implemented on the whole.

I should point out also that there are 3 full pages of PRs, if there is something in there that you think you want (not just happen but anybody) grabbing and testing is the fastest way to get it into the engine for official support. Feel free not to stick to the list I put up.

[Happenstance]

Update on my part of this:

* I ended up rewriting #2168 and #2140 (only for the new templates) and put them each in new, separate branches. Not sure what the best practice is for updating the existing PRs?

* Will test #2076 tomorrow at some point.

* Tested #2054 (Allow Negative Lights) and #2154 (SceneObject::isRenderEnabled() cleanup) as well - no issues!