Me.SpellReady always returns valid indexing

[silverj]

Even when no spell was matched. To correct, add return false; to around line 3918 in MQ2DataTypes.cpp, so it looks like:

Code: Select all

				for (unsigned long nGem = 0; nGem < NUM_SPELL_GEMS; nGem++)
				{
					if (PSPELL pSpell = GetSpellByID(GetMemorizedSpell(nGem)))
					{
						if (!_stricmp(GETFIRST(), pSpell->Name))
						{
							if (((PCDISPLAY)pDisplay)->TimeStamp >((PSPAWNINFO)pLocalPlayer)->SpellGemETA[nGem] && ((PCDISPLAY)pDisplay)->TimeStamp > ((PSPAWNINFO)pLocalPlayer)->SpellCooldownETA) {
								Dest.DWord = 1;
							}
							return true;
						}
					}
				}
				return false;

[silverj]

Same for Me.AltAbilityReady, to fix return false at around line 3534 instead of true.

If this is intended behaviour, I would propose rather only returning false for empty indices, if there are scripts that depend on the return being FALSE for non-existant things, rather than the NULL that I would percieve as a more correct result.

[silverj]

I guess it would be best to retain the any real index returns a value, so a better fix for SpellReady would also be to just change the default return from true to false, and return true at the end of the had-an-index path.

[SwiftyMUSE]

In both cases the code is accurate. The "return true" tells the macro language that the return value is set... ie, true/false as it is a Boolean type. A "return false" does not mean that the spell or ability is not ready, it means that the macro language is not getting a return value from the code.

[silverj]

So which spell is not ready when you index with "" ?

I made lua (a scripting language) interface for mq2, and these empty indices returning a number are special cases, that break the ordinary indexing chain, that would work with everything else, except the few MQ2CharacterType members that do not return any real information on an empty index.

But yeah, whatever, I will work around them, then, if you feel they are something that mq2 scripts need.

[SwiftyMUSE]

So which spell is not ready when you index with "" ?

I made lua (a scripting language) interface for mq2, and these empty indices returning a number are special cases, that break the ordinary indexing chain, that would work with everything else, except the few MQ2CharacterType members that do not return any real information on an empty index.

But yeah, whatever, I will work around them, then, if you feel they are something that mq2 scripts need.

All, since you are not passing any valid data. Yes, we could put special use case in the base code for every single place that invalid data is being passed... OR, the programmer of the script that is running could verify they are always passing valid data. Both are accepted methods to handle this, we just chose the one that is easiest on us to code and maintain.

[silverj]

Actually, you chose inconsistently, some places they return data not present (C++ return false) to get a scripting NULL, while other places they return data present with a FALSE/0 value. This is not maintainability/readability/other excuse in my eyes - but of course it is not my code, so my eyes lie.

For example, AltAbilityTimer returns NULL/invalid while AltAbilityReady returns 0/not ready when indexed with an invalid index. Why? To me it looks like a bug, they are inconsistent when they should be consistent. But as I said, I will just work around them if you feel people have plenty of scripts out there that depend on these inconsistencies. Also as and additional commend, AbilityReady does return the NULL correctly, so you would also need to consider why that would differ from the rest of AbilityReady cases, if you feel this is not an inconsistency issue.

The ones I use that are not like AltAbilityTimer or AbilityReady (which are the right way from consistency perspective - you perform lookups in program order when indexing, maintaining outstanding indices until you find valid data, after which you will continue either evaluation or further indexing based on that valid data), and need to work around are: CombatAbilityReady, AltAbilityReady, ItemReady, SpellReady.

[SwiftyMUSE]

Actually, you chose inconsistently, some places they return data not present (C++ return false) to get a scripting NULL, while other places they return data present with a FALSE/0 value. This is not maintainability/readability/other excuse in my eyes - but of course it is not my code, so my eyes lie.

For example, AltAbilityTimer returns NULL/invalid while AltAbilityReady returns 0/not ready when indexed with an invalid index. Why? To me it looks like a bug, they are inconsistent when they should be consistent. But as I said, I will just work around them if you feel people have plenty of scripts out there that depend on these inconsistencies. Also as and additional commend, AbilityReady does return the NULL correctly, so you would also need to consider why that would differ from the rest of AbilityReady cases, if you feel this is not an inconsistency issue.

The ones I use that are not like AltAbilityTimer or AbilityReady (which are the right way from consistency perspective - you perform lookups in program order when indexing, maintaining outstanding indices until you find valid data, after which you will continue either evaluation or further indexing based on that valid data), and need to work around are: CombatAbilityReady, AltAbilityReady, ItemReady, SpellReady.

Any return that does not send the return type (pTimeStamp, pBoolean, etc...) is not correctly coded. AltAbilityTimer is "bugged" by not being correctly coded, where AltAbilityReady is properly coded. This would require going through the entire TLO source and making sure those that don't set a return type/value are properly coded.

[silverj]

I see I did not manage to explain it clearly enough to be understood. But it is fine, as I worked around it.

But how do I upload a plugin with a few dozen source files into the builder? It says to upload .cpp files, but I have both c/cpp and the project file. I tried to upload a zip, but it says only to upload .cpp files...

And where should the builder discussion take place?