Infrastructure: rationalise strings in TLuaInterpreter class#6636
Conversation
In particular try and use ID in a uniform manner when referring to an integer number that uniquely identifies an instance of something like a room or an area in the map. It became clear that there are a significant number of duplicate strings involving ID and it was possible to reduce the number of duplicate `qsl(`...`)` - which are our wrapper of `QStringLiteral` - by replacing each with a QString constant initialised at compile time. This is a good thing as it is bad to have multiple copies of the same string as one of these as each one is stored separately in the (Read-Only) data part of the executable. This should close Mudlet#2605. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
|
I think this will clash with the Lua API changes in #6615 which will need to be resolved in the second of the pair before it is merged. |
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
vadi2
left a comment
There was a problem hiding this comment.
Centralising these often-repeated strings within the class makes sense. Let's not go crazy with this though - we don't know what the space savings actually are.
Sadly, my testing has proven confusing and disappointing, making a common instance of what would be duplicate identical
What centralising the strings does do is reduce the different number of strings - hopefully no one is coding against the error message text but just using them for debugging when they get a |
Brief overview of PR changes/additions
In particular try and use ID in a uniform manner when referring to an integer number that uniquely identifies an instance of something like a room or an area in the map.
Motivation for adding to Mudlet
It became clear that there are a significant number of duplicate strings involving ID and it was possible to reduce the number of duplicate
qsl(...)- which are our wrapper ofQStringLiteral- by replacing each with a QString constant initialised at compile time. This is a good thing as it is bad to have multiple copies of the same string as one of these as each one is stored separately in the (Read-Only) data part of the executable.Other info (issues closed, discussion etc)
This should close #2605.
This was prompted by @Kebap 's comments here: #6615 (comment)