Consider changing spell_id from int16 to int or int32 about server HOT 14 OPEN

It looks like some packets use 16-bit ints and some use 32-bit ints

from server.

I think we just need to go through and switch all the remaining over to int32 now. I do highly recommend int32 over uint32 for this though.

from server.

But that limits us to 2 billion spells instead of 4 billion! How could we gimp ourselves so badly?

from server.

Not really, RoF2 can't use IDs larger than 45k.

from server.

Clients have hard coded caps

from server.

While I was -clearly- joking, to clarify on the recommendation for int32 instead of uint32, EQ treats SpellID as a signed number in some places, such as spell effects on items, which are -1 if there is no effect in that slot.

from server.

You weren't joking, you were bloody serious

On Sat, Nov 14, 2015 at 4:34 PM, Jon D. Jackson [email protected]
wrote:

While I was -clearly- joking, to clarify on the recommendation for int32
instead of uint32, EQ treats SpellID as a signed number in some places,
such as spell effects on items, which are -1 if there is no effect in that
slot.

—
Reply to this email directly or view it on GitHub
#7 (comment).

from server.

This shouldn't be closed. We still need to move spell IDs all over to a bigger sized int.

from server.

Then someone should change the datatypes quick cause it should be a fairly simple change and test so we can close this out

from server.

If it was so simple, it would be done already. I'm 99% sure I fixed one of the issues I ran into originally, but I just haven't revisted it yet.

from server.

Put your big boy pants on and git r dun

from server.

I looked into some more issues that might arise from fixing this. It looks like even though spell IDs were switched to ints at some point, the clients still used int16_t in the BeginCast_Struct. Luckily, these clients don't support spell IDs beyond the int16_t range, so we can just handle this in translators. (RoF+, at least for us use an int for this packet)

from server.

The CombatDamage_Struct as well as Action_Struct also still use int16_t for spell ID. We'll have to handle this in the translator as well.

Not sure what we should do since this packet has a more significant client side affect ...

from server.

The CombatDamage_Struct as well as Action_Struct also still use int16_t for spell ID. We'll have to handle this in the translator as well.

Not sure what we should do since this packet has a more significant client side affect ...

Was changed to int32 in ROF1.

Clients prior to ROF1 do not support that high of spell values anyway. In the encoder, they should simply be set to unknown spell ID. (-1, or SPELL_UNKNOWN macro - though, we should probably really have the SPELL_UNKNOWN macro reference the int32_t version and use the limits for each expansion)

In fact, I'd go as far as to say we can change the code in npc spells and other related places to int32_t once that is taken care of.

I have this functionality already (mostly) in classless, will have to port it over soon(ish) - the DB should already support int, though I will have to get PEQ and confirm.

References:

https://github.com/EQEmu/Server/blob/master/common/patches/uf_structs.h#L1263
https://github.com/EQEmu/Server/blob/master/common/patches/rof_structs.h#L1461

from server.