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

Auto stack fix #2955

Merged
merged 2 commits into from
Apr 29, 2020
Merged

Auto stack fix #2955

merged 2 commits into from
Apr 29, 2020

Conversation

nekiro
Copy link
Member

@nekiro nekiro commented Apr 21, 2020

Fix #2954

@EPuncker
Copy link
Contributor

tested and working like a charm, I can select any stack in the container to move the item to and it wont always chose the first slot

@yamaken93
Copy link
Member

For a reason i forget i did it differently:

if (index != INDEX_WHEREEVER) {
    Item* itemFromIndex = getItemByIndex(index);
    if (itemFromIndex) {
        if (itemFromIndex != item && itemFromIndex->equals(item) && itemFromIndex->getItemCount() < 100) {
            *destItem = itemFromIndex;
            return this;
        }
    }
}

@nekiro
Copy link
Member Author

nekiro commented Apr 23, 2020

@cristofermartins your code does not consider if itemFromIndex is a cylinder (container) which happens here:

Cylinder* subCylinder = dynamic_cast<Cylinder*>(*destItem);
if (subCylinder) {
index = INDEX_WHEREEVER;
*destItem = nullptr;
return subCylinder;
}

@yamaken93
Copy link
Member

Yeah but no bug will happens since equals won't match the id and moving the item direct to the backpack so it will move inside the backpack stills works.

@nekiro
Copy link
Member Author

nekiro commented Apr 23, 2020

@cristofermartins thats nice, also it does ignore the flag "ignoreautostack"

@yamaken93
Copy link
Member

The code i posted happens below the autostack flag is checked:

bool autoStack = !hasBitSet(FLAG_IGNOREAUTOSTACK, flags);
bool isStackable = item->isStackable();
if (autoStack && item->isStackable() && item->getParent() != this) {
    if (index != INDEX_WHEREEVER) {
        Item* itemFromIndex = getItemByIndex(index);
        if (itemFromIndex) {
            if (itemFromIndex != item && itemFromIndex->equals(item) && itemFromIndex->getItemCount() < 100) {
                *destItem = itemFromIndex;
                return this;
            }
        }
    }

@nekiro
Copy link
Member Author

nekiro commented Apr 23, 2020

@cristofermartins oh I see, so you wrote a bunch of redundant code imo. You have all these things you wrote before that autoStack check and you can just check the destItem variable.

@yamaken93
Copy link
Member

Yeah i think its redundant as you said.

Copy link
Contributor

@MillhioreBT MillhioreBT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems correct, I tried it and it works, if someone else can confirm it it would be much better.

@nekiro nekiro added the bugfix label Apr 27, 2020
@DSpeichert DSpeichert merged commit cbcaaea into otland:master Apr 29, 2020
@nekiro nekiro deleted the stack-fixes branch April 29, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Player::queryAdd always picking first slot
5 participants