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

Hotfix for status 6 (e-mail confirmations) #732

Closed
wants to merge 2 commits into from
Closed

Hotfix for status 6 (e-mail confirmations) #732

wants to merge 2 commits into from

Conversation

scholtzm
Copy link
Contributor

@scholtzm scholtzm commented Mar 1, 2015

This hasn't been fixed yet so I'm opening a PR to get this sorted out ASAP.

There are couple small changes:

  • added support for status 6 in the Trade.cs
  • slightly modified status 2 according to node-steam-trade
  • added support for tradeid in TradeStatus.cs

Currently, the way we handle errors, it's not possible to provide tradeid to be handled directly in the UserHandler. This could be improved.

@scholtzm
Copy link
Contributor Author

scholtzm commented Mar 1, 2015

Ref: issue #708

@BlueRaja
Copy link
Collaborator

BlueRaja commented Mar 1, 2015

I don't think firing the error handler when the trade completes with "email pending" is the correct behavior.

@scholtzm
Copy link
Contributor Author

scholtzm commented Mar 1, 2015

In that case we would need to add new method to UserHandler class since the trade is not completed either.

Ideally, we should have a single "trade ended" handling method which takes TradeStatusType as the only argument. This way the user (bot programmer) can easily decide what to do and doesn't have to decipher arbitrary error string created by the GetTradeStatusErrorString method.

@BlueRaja
Copy link
Collaborator

BlueRaja commented Mar 1, 2015

There doesn't seem to be a good, backwards-compatible solution. So perhaps we should just do that.
@Bottswana @WildCard65 @Jessecar96 @FrozenHaxor What do you guys think?

@WildCard65
Copy link
Contributor

I say we let bot programmer decide what to do on trade end

@Jessecar96
Copy link
Owner

Well I haven't kept up to date that much on SteamBot so I'm not sure if it even supports trade offers natively. It should check for outgoing offers every X seconds (or check on the event for that if there is one) to see when the offer is accepted, then fire the success event, and if it was declined fire the error event.

@FrozenHaxor
Copy link
Contributor

The problem is inconsistency on Valve's part. Mostly the trades appear as "Items now unavailable for trade" and such after being accepted by both parties and sometimes it shows it correctly. They messed it up big time.

@BlueRaja
Copy link
Collaborator

BlueRaja commented Mar 3, 2015

There currently isn't an accurate and exploit-free way to determine if a trade-offer has been completed. We discussed this to death in another thread.

But that's not what we're talking about here - we're talking about what to do when a normal trade gets turned into a trade offer due to an email-confirmation.

@scholtzm
Copy link
Contributor Author

scholtzm commented Mar 4, 2015

@BlueRaja Adding to my first reply to you, I just realized that if the proposed OnTradeEnded method accepted TradeStatusType as the only parameter, the tradeid value from the TradeStatus class would not be available. So it would be probably better to include both or if you want to simplify things, only the last received TradeStatus object (which contains trade_status other than 0).

@scholtzm
Copy link
Contributor Author

scholtzm commented Mar 6, 2015

BTW. One more thing I have noticed. Trades sometimes end with status 6 even if both users have the e-mail confirmations disabled. The items are exchanged immediately though.

@FrozenHaxor
Copy link
Contributor

Trades sometimes end with status 6 even if both users have the e-mail confirmations disabled. The items are exchanged immediately though.

I confirm this. It's actually happening quite often.

@BlueRaja
Copy link
Collaborator

BlueRaja commented Mar 6, 2015

When that happens, are we given a trade-offer id we can check? Does that trade offer immediately complete successfully, or does it sometimes conclude with "Items not available" like normal trade-offers sometimes do?

@scholtzm
Copy link
Contributor Author

scholtzm commented Mar 6, 2015

The trade offer is instantly marked as "Items unavailable". Another issue which makes the system completely unreliable.

@BlueRaja
Copy link
Collaborator

Before this gets merged, I think this PR should either create the OnTradeEnded callback suggested above and remove the existing overlapping callbacks (which would be a breaking change), or just add a new callback when there's an email confirmation (which wouldn't be a breaking change, but seems slightly excessive, since a trade could now end with three separate callbacks).

@BlueRaja
Copy link
Collaborator

I added a new callback: #746

@scholtzm
Copy link
Contributor Author

I'm closing this PR. Discussion continues in #746.

@scholtzm scholtzm closed this Mar 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants