More drive-by cleanup

[AudriusButkevicius]

No description provided.

[Lectem]

Thanks again for the PR.
A few changes and good to go !

[Lectem]

Comments are no longer aligned (tabspace = 4), this can not be seen by default as github uses 8 spaces for tabs.
I really need to get some .clang-format setup, but it quickly messes up with array constants so I need to investigate more before doing so...

Note for myself in the future: this can be changed at the user level in https://github.com/settings/appearance

[AudriusButkevicius]

I vaguely remember github respecting .editor or .editorconfig in the root of the repo for display purposes.

[AudriusButkevicius]

Indeed:
isaacs/github#170 (comment)

[Lectem]

It's kind of unfortunate we have to add yet another file for this :(
But I guess this is better than having to explain the issue every time.

[AudriusButkevicius]

Given:

return D2GAME_NPC_BuyItemHandler_6FCC92A0(pGame, pUnit, *(int32_t*)((char*)pPacket + 1), *(int32_t*)((char*)pPacket + 5), (*(int32_t*)((char*)pPacket + 9) & INT_MAX) >> 16, *(int32_t*)((char*)pPacket + 9), *(int32_t*)((char*)pPacket + 13), *(int32_t*)((char*)pPacket + 9) & 0x80000000);

This field is a bit more interesting, as sometimes it reads just the top bit, sometimes, the 15 upper bits minus the top one, sometimes the whole thing, however I think making struct/union/bitstruct for this might be annoying.

[Lectem]

I think we may want to have a union or at least helper functions in this case. After a quick look I think those 4 bytes pack the following information:

• Transaction type bits 0 - 15 (enum should be in D2Common, as it must be used in ITEMS_CalculateTransactionCost)
• Something related to max cost for item transaction ? Seems to mostly be used with value of 0. ItemMode, but should always be 0? bits 16-30
• Fill item (book, pots, scrolls...) bit 31

[Lectem]

Transaction types are:

• 0 buy
• 1 sell
• 2 gamble
• 3 repair

[AudriusButkevicius]

Not sure that is correct, as its a buy packet, so sell doesn't make sense. I also assumed there is a separate repair packet.

[Lectem]

Well, to be fair, values 1 and 3 are unused, but the field is converted and used as transaction type, which may hold any of the 4 values mentioned above.
I suppose we could have a specific enum for the packet, then cast to the real transaction type enum.

[Lectem]

As for the issue of the packing, I just checked and it's packed because they use the same function for all packets of size 0x11 (D2Client.0x6FAADAA0) which takes 4 DWORD params.

For reference here is how it is computed:

        nPackedTransationInfo = (unsigned __int16)nItemAnimMode << 16;
        if ( gbVendorGamble_6FBB5D7C )
            nPackedTransationInfo |= 2u;
        if ( (nKeys & 4) != 0 )                 // is Shift held
        {
            nClassId = pItem ? pItem->dwClassId : -1;
            pItemRecord = D2COMMON_10600_GetItemTxtRecord(nClassId);
            if ( pItemRecord )
            {
                if ( pItemRecord->nMultibuy )
                    nPackedTransationInfo |= 0x80000000;
            }
        }
        D2CLIENT_PACKETS_SendPacketSize17_6FAADAA0(
            0x32,
            gnActiveNpcGUID_6FBB5CF5,
            dword_13E25BD5,
            nPackedTransationInfo,
            a5);

[AudriusButkevicius]

It's a bit annoying that I can't do the following:

	struct {                                //0x0B
		uint16_t nItemMode : 15;
		uint8_t bFill : 1;
	};

seems the compiler insists on padding the uint16_t to 16 bits, at which point bFill causes an extra byte.

I don't think the 7 bits are actually used for anything, so I'm just dropping them on the floor.

[Lectem]

This is because you are changing the type.
I'm guessing

struct {                                //0x0B
		uint16_t nItemMode : 15;
		uint16_t bFill : 1;
	};

would work.

[AudriusButkevicius]

Learned something new. I could also now do something like:

	struct {
		D2C_TransactionTypes nTransactionType : 15;
		D2C_ItemModes nItemMode : 15;
		uint32_t bFill : 1;
	};

and then change signatures of D2GAME_NPC_BuyItemHandler_6FCC92A0 and below to use enum types that are bigger.
In reality the original functions have 2 byte types, but I think passing 4 bytes values might still work?
Perhaps it might not work if the values are passed on the stack depending on the calling convention?

[AudriusButkevicius]

or maybe it would be ok:
https://stackoverflow.com/questions/45127993/how-many-bytes-does-the-push-instruction-push-onto-the-stack-when-i-dont-specif/45128435#45128435

[Lectem]

I think the current version is good enough, putting enums in a bitfield will be ambigous/risky (different types might trigger new bitfield). I think it is good to have the uint16_t so that it is explicit that we are packing something unrelated into 16bits.

[Lectem]

Is the last commit supposed to add .editorconfig ? Or was it only to change the indentation type ?

[AudriusButkevicius]

It was a whinge that there isn't one, and I had to fix up spaces vs tabs :)