search

nathanj / spirit-island-pbp

Spirit Island PBP Helper
4 stars 7 forks source link

Add PR for allowing technical support for multi element cards #39

Closed TheOriginalSoni closed 1 month ago

TheOriginalSoni commented 3 months ago

The base game does not support cards with +2x air but it's very easy to technically execute.

Just need

https://github.com/nathanj/spirit-island-pbp/blob/951fc130073fa6774a2ea5e5c1d8b478f5e06ad7/pbf/models.py#L102 change that = 1 to a += 1

TheOriginalSoni commented 3 months ago

This and some other edits were already merged in https://github.com/TheOriginalSoni/spirit-island-pbp/ but no longer are part of the commits here. Some revert shenanigans?

samsullivan commented 1 month ago

I vaguely remember messing with this line when helping setup your fork; but, I can't remember what the problem was and don't think I understand from this issue's description?

petertseng commented 1 month ago

I strongly advise against adding this feature to this, the main instance of the site. Other instances which support custom content can do what they want, but not here.

The reason is two principles:

Currently there is no published material that can have a card with more than one of any element. If someone accidentally creates one in the data, the site should loudly reject it (even going so far as refusing to start until the error is corrected would be acceptable behaviour) instead of silently doing the wrong thing.

petertseng commented 1 month ago

I'll submit something that does the check I described, but adds comments that aim to make it clear what should be changed if a future expansion adds such content (or an instance of the site that supports custom content wants to support it)

samsullivan commented 1 month ago

The base game does not support cards with +2x air

For now, this repo has focused on what the official game (+ official unreleased content) offers; in my opinion, this is probably best for a fork, despite it being an easy fix as you say.

If we get to a scenario where people are uploading custom cards for custom games on the core PBP, I'd be willing to reconsider. (Right now I see 342 cards in the production DB; the Wiki says 334 including retired Growth Through Sacrifice plus 4 * 2 for the Apocrypha spirits.)

Especially since you can't add cards at this point, only change them (which would affect every game on the server) which is arguably not really worth it when a fix should be made through a migration here.