Add optional CommanderBracket.app integration for bracket estimation#10948
Add optional CommanderBracket.app integration for bracket estimation#10948Madwand99 wants to merge 3 commits into
Conversation
tool4ever
left a comment
There was a problem hiding this comment.
Sorry, I liked your offline implementation but I'm not really a fan of such logic to simply connect with a third-party API, besides needing all this massive code hardly being reasonable trade-off
- only to maybe get a more accurate value
- save a few mouse clicks for the user to just use his browser instead
(And Scryfall alone already causes enough headache)
That’s fair, and I understand the concern. I agree that Forge should be cautious about third-party services, especially given the ongoing Scryfall maintenance burden. The reason I think this case may still be worth considering is that the offline implementation and the CommanderBracket.app result are solving different problems. Forge’s local calculation is intentionally a minimum bracket estimate. It is useful as a floor, but it cannot give the kind of holistic bracket estimate Commander players usually want: deck speed, tutors, combo density, game changers, archetype context, and narrative reasoning. We could try to port that logic into Forge, but that would likely be much larger than this PR and would also mean maintaining the model ourselves as Commander bracket guidance evolves. This PR is intended as a compromise: keep Forge’s lightweight offline estimate, while optionally allowing users to ask CommanderBracket.app for a better estimate when they want one. I also tried to limit the blast radius:
So I don’t see this as replacing Forge logic with a web dependency. I see it as giving users an optional bridge to a specialized bracket estimator, while keeping Forge’s existing offline behavior intact. The real question may be whether Forge wants a more accurate Commander bracket estimate at all. If yes, then I think either we maintain a much larger model in Forge, or we use an optional external integration like this. My feeling is that this PR is the smaller and more maintainable version of that feature, but I’m open to reducing the scope further if there are specific parts that feel too invasive. |
This adds an optional CommanderBracket.app integration for richer Commander bracket estimates while keeping Forge’s local bracket calculation as the default fallback.
Key changes:
Thanks to Codex for help with code and keith@commanderbracket.app for help with API access!