How to have a variable list of targets for each card

[FastEddie]

This topic came up over debugging Mimic Vat. For more background see here: https://www.slightlymagic.net/forum/viewtopic.php?f=86&t=23213
 
Goal is to have a variable list of targets attached to each card instance instead of the previous 9 (19 in fact but the upper 10 are used for something else) as having a potentially unlimited number of targets but only limited storage is a surefire recipe for trouble.
 
For more details on targets see here: https://www.slightlymagic.net/forum/viewtopic.php?f=56&t=29883
 
Since this is a major change of the existing code and must be backward compatible to boot, I wanted to discuss and brainstorm it here before writing a single line of code. Comments, suggestions, etc. are welcome.
NB: I try to write this in layman’s terms for ease of understanding and simplicity. If something is unclear or just sounds weird please let me know.
 
Backward compatibility:
This could be achieved if we store a reference to our list of targets within the upper targets, i.e. deprecate and re-use one of the upper targets between 10 and 19. 11 has been named as a candidate. This would give us 64 bits of space to play with.
Existing code assumes that targets are given in entries 1 to 9, so the previous behavior should not change. Functions must actively look for the new list.
It would be ideal if we could shift target 19 to 11 after deprecating 11 (i.e. rewrite existing code using target 19). Reason is that we could shorten the array to 18 entries and have a new entry after it. Otherwise we would have to make the target_t structure a union containing

Code: Select all

{unit32_t player, unit32_t card}

and our new

Code: Select all

{64-bit reference with type to be determined}

or alternatively

Code: Select all

{32-bit reference, uint32_t padding}

and I don’t know what the compiler does in that situation and what other can of worms we open with that. A separate variable would be safer.
 
Storage (global or local):
This is the crucial topic. There are two alternatives in my opinion, globally for all cards or locally attached to each card instance.
 
Globally:
• Would probably be a global list of lists list<list<target_t>>, the outer list containing the reference (card iid maybe), the inner containing the card’s targets.
• Would need access control and therefore probably be a Singleton pattern (yes, I know, the world would be a better place if people would use less of them).
• Would allocate memory for the list on the stack, so no messing with memory allocation which would be positive.
 
Locally:
• Would be a list list<target_t> with a pointer towards it stored in the card instance.
• Every time a card “comes to life” (to my understanding when the card instance is created or when it is added to cards_data[]?) the memory for the list must be allocated and the pointer be set. When the card “dies” the memory must be freed, and the pointer set to NULL or something similar. I have no idea where this could happen, maybe the rules engine provides an entry point.
• As every card has its own list (e.g. Damnation would have a list of destroyed creatures and tokens) access control shouldn’t be an issue.
 
I would prefer the local solution given that we find a way to allocate and free the memory properly. Once the list has been created adding elements is done by list.push_back() or list,push_front(), meaning that we don’t have to care about memory allocation.
 
Either way we need to handle exceptions that might be thrown by the list container. This is probably a job for the single card or the global storage.
 
After usage, the list must be cleared by calling list.erase(list.begin(), list.end()) to remove all stored targets, otherwise they will be around next time the card is called (not an issue with Damnation I think but a Regrowth could already cause issues… clearly an issue with permanents storing targets for some time). I don’t know when this would be done, though.
 
Reference to stored object:
Would be either a unique id to the card (probably the card iid) in case of the global solution or a pointer in case of the local solution. Card iid is only 32 bit, so we have another uint32_t space to play with just in case.
 
Usage:
A card that doesn’t know about this mechanism wouldn’t do anything different, i.e. loop through targets 1 to 9 and do its thing.
A card that knows would either ask the global storage whether targets are stored there (and if not probably fall back to looping through the targets 1 to 9) or check whether the pointer is not NULL. If so, it can go through the list and do its thing. This could be a chance to clean up the list (deleting each entry after taking it) in case the list of targets is not used by another card afterwards. No idea whether this is the case or not, but it would be cool if we could do that here.

[FastEddie]

So I had some offline discussion in the meanwhile and learned that putting a raw pointer inside the structure is not a good idea as well as that the whole thing is way more involved than I imagined. Why means that this is shelved for the time being.