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

Two unrelated item events for same person can occur at same millisecond #280

Open
deklanw opened this issue Mar 14, 2019 · 3 comments
Open

Comments

@deklanw
Copy link

deklanw commented Mar 14, 2019

If I'm not mistaken the logic here relies on the assumption that every separate item event for any given participant has its own timestamp (except items being destroyed when upgrading items):
https://github.com/meraki-analytics/cassiopeia/blob/master/cassiopeia/core/match.py#L887

As far as I can tell, it's a freak occurrence but see: BR1 1594528336

Look at some of this person's item events:

{
          "type": "ITEM_PURCHASED",
          "itemId": 3303,
          "timestamp": 7830,
          "participantId": 3
        },
        {
          "type": "ITEM_PURCHASED",
          "itemId": 2003,
          "timestamp": 14854,
          "participantId": 3
        },
        {
          "type": "ITEM_UNDO",
          "afterId": 0,
          "beforeId": 3303,
          "timestamp": 14854,
          "participantId": 3
        },
        {
          "type": "ITEM_PURCHASED",
          "itemId": 2003,
          "timestamp": 15151,
          "participantId": 3
        },
        {
          "type": "ITEM_UNDO",
          "afterId": 0,
          "beforeId": 2003,
          "timestamp": 16405,
          "participantId": 3
        },
        {
          "type": "ITEM_UNDO",
          "afterId": 0,
          "beforeId": 2003,
          "timestamp": 16900,
          "participantId": 3
        }

If I'm not wrong, Cassio assumes that any item event by a participant that has occurred between an item event and its undo must be components being destroyed. That isn't the case here.

For example, in this case, I believe when they try to undo the Spellthief's purchase Cassio will delete the Pot (as if it were a component). Later, when the person tries to undo the Pot purchase, it's not there.

This is much easier to see if you imagine moving the Spellthief undo event to after the Pot purchase event (as it should be).

@jjmaldonis
Copy link
Member

Without looking into it much, this seems pretty plausible. You said, "If I'm not mistaken the logic here relies on the assumption that every separate item event for any given participant has its own timestamp" but I believe the while loop doesn't depend on timestamps: while prev is None or prev.item_id != item_id:

Maybe if two events get recorded at the same time, the order gets messed up in Riot's system, which would cause a dependence of the while loop on the timestamp. If so, the data we get from Riot is incorrect and it's probably unreasonable to expect Cass to fix it.

What are the chances you have a solution?

Things are pretty busy right now, and since this is a rare occurrence and the cumulate_timeline functionality is largely an accessory, I'm inclined not to be in a hurry to fix it unless someone else comes up with a good solution.

@deklanw
Copy link
Author

deklanw commented Mar 17, 2019

I'm sure it's possible to fix it in Cass. I'm just not sure of an elegant solution.

One way would be to do a custom sort on all events: by timestamp, and on equal timestamps always put the UNDO events first. (assuming several UNDO events can't occur at one timestamp....)

Also, found another breaking game fwiw 235386361 OCE

@jjmaldonis
Copy link
Member

I have a feeling that if the issue is network lag, that several UNDO events can occur at one timestamp. I'm not sure we can rule it out at least.

If we did a custom sort on all events and on equal timestamps always put the UNDO events first, would that mean that the event that was being undone could be sorted into a position after the UNDO event itself? I think that would be possible.

Because of these issues, my current inclination is to call this a "#wont-fix". Opinion?

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

No branches or pull requests

2 participants