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 custom diffs #10

Open
KingSupernova31 opened this issue Dec 25, 2022 · 2 comments
Open

Support custom diffs #10

KingSupernova31 opened this issue Dec 25, 2022 · 2 comments

Comments

@KingSupernova31
Copy link

Right now the diffs only go back to Conspiracy, and they only cover a single set of changes. Instead, let users choose any two sets, and it generates a diff between them. This would be very useful for anyone getting back into Magic after taking a break, since they could look at a single diff covering all the changes since they left rather than needing to go through several individual ones. It would also simplify the UI, since you'd no longer need to display a long list of all available diffs.

@lunakv
Copy link
Owner

lunakv commented Dec 30, 2022

There are plans to extend the diffs further back at some point. The reason they stopped at CON is mostly that starting with Born of the Gods, the text files have some really scuffed formatting, and while that's definitely not an insurmountable problem, I didn't really have time to deal with it before launch.

As far as dynamically generated diffs, that's something I remember discussing with Vill years ago, I think he even spent some time trying to implement it, but ultimately scrapped the idea. Some of my thoughts as to why I'm reluctant to try it at the moment:

  • Once you get more than a couple of sets apart, my guess is that the diffs would get so large as to be basically useless.
  • I don't think the value proposition is quite there. You mention players getting back into the game as a potential demographic, but I don't think any regular players reading the CR, much less CR diffs, so I don't see the draw there.
  • I disagree that it would simplify the UI. I'm no UX designer myself, but coming up with dynamic elements to select and render arbitrary diffs on the fly doesn't seem trivial, and conversely a list is one of the simplest UI components I can think of.
  • It would require a lot of extra testing. The diff engine is calibrated right now to produce mostly accurate diffs when used on two adjacent sets, but I have no idea how it would behave when run on sets far apart. That would mean I'd have to either manually verify every possible combination of diffs, which I really don't want to do, or I'd have to just take it on trust that the engine is working correctly, which I'm not comfortable with.
  • Related to the previous point, each new set would now mean O(N) new possible diffs, which makes the maintenance much more time-intensive for me as well.

All in all, doing this, and especially doing this right, would be a sizeable effort in return for a benefit that I'm not sure I really see. I might change my mind in the future, but for now this is nowhere near my list of priorities.

@KingSupernova31
Copy link
Author

I don't think the value proposition is quite there. You mention players getting back into the game as a potential demographic, but I don't think any regular players reading the CR, much less CR diffs, so I don't see the draw there.

I was more thinking judges, but I agree it's a very small demographic regardless. Agreed that this seems likely not worth the benefit overall.

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