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

fix crate dragging with traders #3548

Merged
merged 3 commits into from
Feb 1, 2021
Merged

Conversation

zjdtmkhzt
Copy link
Contributor

@zjdtmkhzt zjdtmkhzt commented Jan 30, 2021

About the PR

Apparently this has been broken for a while?
There is a pinned issue for it, to be honest, I never see those because I never look to the pinned issue area.
fixes #2061
@pali6 writes that it should be it's own proc, honestly, I don't see the need for that, it seems like a simple typecheck to me but what do I know.
Anyway, even if the way I fixed it is bad, it's probably better than leaving it broken for another 5 months.^^

Why's this needed?

It's a pretty cool feature!

Changelog

(u)zjdtmkhzt
(+)Dragging crates onto traders to sell their contents should now work again for all types of items.

@zjdtmkhzt zjdtmkhzt added the P-Minor A bug that does not impact usage of a feature. These are often visual issues label Jan 30, 2021
@zjdtmkhzt zjdtmkhzt requested a review from pali6 January 30, 2021 23:17
@zjdtmkhzt
Copy link
Contributor Author

Is the issue something like white weed vs normal weed?
In that case we could either just take care with the order in which commodities are stored, or we first check for exact matches and then just types.
Not sure if we already have a thing like that for regular trades? I didn't see one.

@zjdtmkhzt
Copy link
Contributor Author

Yeah, lol, don't merge this yet, right now you get the money for all the types your item applies to.^^

@zjdtmkhzt zjdtmkhzt added the E-DNM [Dev Only] Do Not Merge - can only be removed by the applier or with their explicit permission. label Jan 30, 2021
@zjdtmkhzt zjdtmkhzt removed the E-DNM [Dev Only] Do Not Merge - can only be removed by the applier or with their explicit permission. label Jan 31, 2021
@zjdtmkhzt
Copy link
Contributor Author

ok, done, moved it to its own proc!

code/modules/economy/trader.dm Outdated Show resolved Hide resolved
code/modules/economy/trader.dm Outdated Show resolved Hide resolved
@zjdtmkhzt zjdtmkhzt requested a review from ZeWaka February 1, 2021 16:55
@pali6 pali6 merged commit 5efb75b into goonstation:master Feb 1, 2021
github-actions bot pushed a commit that referenced this pull request Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-Minor A bug that does not impact usage of a feature. These are often visual issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trader click-drag doesn't work.
3 participants