Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔧 [Support] could you help explain why no break there? // no break #2720

Open
william19831103 opened this issue Aug 18, 2024 · 1 comment
Labels

Comments

@william19831103
Copy link

void Item::SaveToDB(bool direct)
{
uint32 guid = GetGUIDLow();
switch (uState)
{
case ITEM_CHANGED:
{
static SqlStatementID updGifts;
if (HasFlag(ITEM_FIELD_FLAGS, ITEM_DYNFLAG_WRAPPED))
{
SqlStatement stmt = CharacterDatabase.CreateStatement(updGifts, "UPDATE character_gifts SET guid = ? WHERE item_guid = ?");
stmt.PExecute(GetOwnerGuid().GetCounter(), GetGUIDLow());
}
}
// no break

@DeltaF1
Copy link
Contributor

DeltaF1 commented Sep 13, 2024

I'm going to assume you are familiar with switch statement fall-throughs. If not I recommend looking up that term since it's kind of an unintuitive part of C-style switch statements (or at least I found it unintuitive when I first learned it!) if you aren't already in the headspace of GOTOs and JMP instructions

The code path for ITEM_CHANGED vs ITEM_NEW is mostly the same (Other than using UPDATE vs REPLACE INTO) so letting it fall through into case ITEM_NEW means there's no code duplication. Regardless of whether an item is newly created or already exists in the database, saving it requires collecting all the information about it and sending it to the database so the two cases should be very similar. I'm not sure exactly why the gifts table has to be updated only in the ITEM_CHANGED state, but if you take a look in ItemHandler.cpp here

item->SetUInt32Value(ITEM_FIELD_FLAGS, ITEM_DYNFLAG_WRAPPED);
item->SetState(ITEM_CHANGED, _player);
if (item->GetState() == ITEM_NEW) // save new item, to have alway for `character_gifts` record in `item_instance`
{
_player->SaveInventoryAndGoldToDB();
}

you can see a comment that talks about needing to update that table after wrapping. The players inventory is saved to the database (thus calling Item::SaveToDB) as part of wrapping a gift, so depending on the behaviour of SetState I think this just exists for the gift wrapping case.

Addendum on SetState

SetState gets called and then GetState right afterwards. If SetState were just a simple setter then this branch would never be taken since the state was just set to ITEM_CHANGED and so should never be ITEM_NEW. Looking into SetState, if the item is in the NEW state then it doesn't actually change its state to ITEM_CHANGED until later on so this seems to be fine and intended.

if (state != ITEM_UNCHANGED)
{
// new items must stay in new state until saved
if (uState != ITEM_NEW) uState = state;
AddToUpdateQueueOf(forplayer);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants