Port to Qt 6 - #21
Conversation
c4a4322 to
c422228
Compare
c422228 to
2fef136
Compare
|
I'm not familiar at all with this codebase, and not even that experienced with C++, and I'm very foggy today, so please feel free to comment/critique and ill change it to get this PR merged! :D |
|
Nice work. I would upload this to debian/experimental as a patch if I'm really bored or anyone ask for ;-) |
|
Any new uploads to Debian regarding TADS would be very nice. Maybe they
wouldn't think TADS is obsolete.
Thanks, Jeff ( Fos1)
…On Wed, Nov 15, 2023 at 5:11 AM Alexandre Detiste ***@***.***> wrote:
Nice work. I would upload this to debian/experimental as a patch if I'm
really bored or anyone ask for ;-)
—
Reply to this email directly, view it on GitHub
<#21 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADDAWSLR6C4RIJH4UU2L6HLYESPPDAVCNFSM6AAAAAA6UEY4S2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJSGMYTCMZWG4>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
it is very well alive, I enabled CI right now. one can have a look a the build warnings if interrested https://salsa.debian.org/games-team/qtads/-/commit/a8cf846ac80918031a37c3a33da8981cf506cec7 |
|
Thank you very much for doing the work! I'll need to find some time go over it and merge it. Right now I'm kind of swamped with other things. |
Thank you! And yeah that's perfectly understandable. |
|
Any status update? At least Gentoo and Ubuntu are now pursuing Qt5 removal trackers. |
I'm still around to work on any changes the author needs me to make (although I've been flaking on a number of my contributions to open source projects lately because work has been absolutely hammering me so there may be some delay, but my attention is still available at least). The bottleneck is just whenever he has time. |
Qt5 is now declared deprecated in GentooAt this point in time, Linux distributions are already tracking which packages are blocking Qt5 cleanup. Real Qt Company OSS support stopped back in 2020, when public availability of LTS releases started to be delayed for 1 year, stable branch repository access got closed down (with 5.15 branch repository receiving any new LTS tag one year after commercial release) and only Qt6-relevant bugs are being fixed (then backported) since then. The final release happened on 2025-05-19, which means a) we will not get it until 2026-06-19, b) any bugs discovered since last May will not be fixed and c) no bugs since 2020-12-08 that haven't also been affecting Qt6 were fixed at all. At least Gentoo, Alpine and Ubuntu are entertaining Qt5 porting trackers by now, for Gentoo, Qt5 revdep cleanups are in full swing already. |
ferdnyc
left a comment
There was a problem hiding this comment.
Thanks for doing this @alexispurslane!
| const QStringConverter::Encoding codecs[9]{ | ||
| QStringConverter::Encoding::System, QStringConverter::Encoding::Latin1, | ||
| QStringConverter::Encoding::Utf16, QStringConverter::Encoding::Utf16BE, | ||
| QStringConverter::Encoding::Utf16LE, QStringConverter::Encoding::Utf32, | ||
| QStringConverter::Encoding::Utf32BE, QStringConverter::Encoding::Utf32LE, | ||
| QStringConverter::Encoding::Utf8, | ||
| }; |
There was a problem hiding this comment.
I don't think this list actually makes much sense. As the comment in the removed code said, in the original set only UTF-8 was included (despite even that not being common in practice) because at least TADS 2 has a one-byte-per-character format. So UTF16 is unlikely-to-impossible, and I don't think I've ever seen UTF32-encoded... anything. Anywhere. Ever.
On my (Fedora Linux) system, presumably thanks to the ICU library, QStringConverter::availableCodecs() produces a list of 233 codecs, pretty much a drop-in replacement for QTextCodec::availableCodecs(). Filtering that list by the same criteria as the old code leaves these 78:
['UTF-8',
'ISO-8859-1',
'ISO-8859-2',
'ISO-8859-3',
'ISO-8859-4',
'ISO-8859-5',
'ISO-8859-6',
'ISO-8859-7',
'ISO-8859-8',
'ISO-8859-9',
'ISO-8859-10',
'ISO-8859-13',
'ISO-8859-14',
'ISO-8859-15',
'EUC-JP',
'EUC-KR',
'windows-874-2000',
'IBM437',
'IBM775',
'IBM850',
'IBM852',
'IBM855',
'IBM857',
'IBM00858',
'IBM860',
'IBM861',
'IBM862',
'IBM863',
'IBM864',
'IBM865',
'IBM866',
'IBM868',
'IBM869',
'KOI8-R',
'KOI8-U',
'windows-1250',
'windows-1251',
'windows-1252',
'windows-1253',
'windows-1254',
'windows-1255',
'windows-1256',
'windows-1257',
'windows-1258',
'ISO-2022-JP',
'ISO-2022-JP-1',
'ISO-2022-JP-2',
'ISO-2022-KR',
'ISO-2022-CN',
'ISO-2022-CN-EXT',
'IBM037',
'IBM273',
'IBM277',
'IBM278',
'IBM280',
'IBM284',
'IBM285',
'IBM290',
'IBM297',
'IBM420',
'IBM424',
'IBM500',
'IBM-Thai',
'IBM870',
'IBM871',
'IBM918',
'IBM1026',
'IBM1047',
'IBM01140',
'IBM01141',
'IBM01142',
'IBM01143',
'IBM01144',
'IBM01145',
'IBM01146',
'IBM01147',
'IBM01148',
'IBM01149']That's probably excessive, but OTOH some legacy codecs like windows-1250, windows-1252, ISO-8859-15, etc. were extremely common in older files, and still come up fairly often even today.
| if (ui->encodingComboBox->findText(QString::fromLatin1(codec)) == -1) { | ||
| ui->encodingComboBox->addItem(QString::fromLatin1(codec)); | ||
| if (ui->encodingComboBox->findText( | ||
| QString::fromLatin1(QStringConverter::nameForEncoding(codec))) |
There was a problem hiding this comment.
According to QStringConverter's docs, nameForEncoding() outputs a const char * string that's UTF-8 encoded, so fromLatin1() sounds like the wrong interpretation. It's probably best to just drop it, since QString will default to interpreting char * as UTF-8 anyway.
(fromLatin1() is used to override that default when dealing with strings that can be processed faster as 8-bit Latin1. But the names returned by nameForEncoding() aren't guaranteed to be 8-bit fixed-width.)
| gameFileName = args.at(1) + ".t3"; | ||
| } else { | ||
| qWarning() << "File" << args.at(1) << "not found."; | ||
| if (args.size() >= 1) { |
There was a problem hiding this comment.
Suggestion: Replace all of this tedious manual command-line parsing with,
#include <QCommandLineParser>
// ...
auto main(int argc, char** argv) -> int {
QString gameFileName;
bool embedOption = false;
// ...
QCommandLineParser parser;
parser.setApplicationDescription("QTads: Interpreter for TADS 2 and 3 game files");
parser.addHelpOption();
parser.addVersionOption();
parser.addPositionalArgument("FILE", "Game file to load");
parser.addOptions({
{{"e", "embed"},
"Print the QTads window ID to stdout on startup (for embedding)"},
});
parser.process(app);
const QStringList args = parser.positionalArguments();
embedOption = parser.isSet("embed");
if (args.length() > 0) {
gameFileName = checkFileName(args.at(0));
if (gameFileName.isEmpty()) {
qWarning() << "File" << args.at(0) << "not found.";
}
}(And a helper checkFileName, something like:)
auto checkFileName(const QString& input) -> QString {
QStringList fileExtensions;
fileExtensions << QLatin1String("")
<< QStringLiteral(".gam")
<< QStringLiteral(".t3");
for (auto& ext : fileExtensions) {
QString name = input + ext;
if (QFile::exists(name)) {
return name;
}
}
return nullptr;
}Qt will handle processing of -h / --help, -v / --version, failing with a message when given unrecognized arguments, and all of the other tedium involved in command-line parsing. When called with --help / --h it'll output something like this:
$ qtads --help
Usage: qtads [options] FILE
QTads: Interpreter for TADS 2 and 3 game files
Options:
-h, --help Displays help on commandline options.
--help-all Displays help including Qt specific options.
-v, --version Displays version information.
-e, --embed Print the QTads window ID to stdout on startup (for embedding)
Arguments:
FILE Game file to loadThe Windows version will even show the help in a window, if QTads is launched from a script or desktop shortcut with a bad argument, instead of in a shell.
In the interests of future maintainability and compatibility, I ported QTADS to Qt6. Everything should work identically, I just went through and replaced uses of constructs that were removed in Qt6. I also removed the uses of
globalStrutentirely because according to the Qt developers it was useless and usually set to zero anyways.I also add the capacity to accept command line flags and two flags (
--helpand--embed), because I need the ability to tell qtads to print out its window id for something else I'm working on. I hope you don't mind.