Forge version 1.5.29

[Chris H.]

Tentative target release date: Friday October 17 2014.

[drdev]

As I mentioned in the new Beta thread, I committed a fix for Commander just a few minutes too late to get in. Is it enough to just have a snapshot build for those who want to play Commander, or should a new Beta build be done?

[timmermac]

Since snapshots are automatically released to Krazy's website, I think directing people there should be sufficient.

[Fizanko]

As I mentioned in the new Beta thread, I committed a fix for Commander just a few minutes too late to get in. Is it enough to just have a snapshot build for those who want to play Commander, or should a new Beta build be done?

Thanks for mentionning this, i'll grab the snapshot instead of beta release.

[Xitax]

Late, but here are KTK booster images.

[drdev]

I just committed the first stage of my large game view refactoring with r27837. This doesn't really change anything yet, other than introducing new refactored *View classes that aren't used yet and cleaning up some other game code. I just wanted to commit it to ensure I haven't broken anything yet and to reduce the chances of merge issues. So if you're working on anything right now, I definitely encourage you to update your source as soon as possible to pull in my changes.

The main thing that's changing is that the View objects will now be stored in forge.game and will be located within the actual game object classes like Card. So card.getView() will return the CardView object associated with that object. What this allows is doing optimized updates of specific properties at the earliest possible opportunity right from the game code, avoiding updating all view properties unnecessarily at the cost of performance, or worse, updating properties too late for the GUI to reflect it due to race conditions.

What I've also done towards the end of adding network support is make these view classes inherit from a new base class called TrackableObject<E>, where E is a special *Prop enum (e.g. CardProp) that will exist alongside each class, containing one enum value for each property in that View class. That base class stores all properties in a single EnumMap<E, Object>, using special protected get(E key) and set(E key, Object value) methods to allow the View class to get and set individual properties. Where this gets cool is that I'm able to maintain a separate EnumSet<E> of all properties that have been modified by a set(), and I can use that EnumSet later to optimize network communication by only serializing the properties that have changed, plus it reduces the size of the serialization by letting us simply send an integer for the enum key rather than the full property name. I'll also be able to send integers in place of View objects themselves, which will avoid issues of serializing a List<CardView> contained with a PlayerView for instance.

I'm definitely interested in feedback on my approach or if people have any additional ideas or concerns. Also, please let me know if you see any ill effects of my changes so far. Next up will be swapping out the refactored View classes for the current View classes, which will hopefully fix all the update problems and improve performance.

Thanks.
-Dan

EDIT: So I just committed another large set of code changes to remove all the places where IGuiBase instances were being stored and passed as parameters. This interface was never intended to be used this way when I created it, and it made the code a lot messier. Ultimately it wasn't going to solve anything in terms of processing network communication either, so I just changed everything back to using GuiBase.getInterface().

With all these changes in mind, please make sure you've updated to a least r27838 before working on anything new.

[elcnesh]

Wow, that a huge change of structure from what I originally came up with... I hope you didn't mind too much to have to change back the IGuiBase references, now that I have a more complete picture of the situation I agree this is the better solution. And the Tracking of objects looks gorgeous, that should really help when sending data over network!

PS: I've been quite inactive for a while because my job just got a whole lot busier, which probably stays true for the coming weeks as well... I'll see what I can do in between working hours

[drdev]

Wow, that a huge change of structure from what I originally came up with... I hope you didn't mind too much to have to change back the IGuiBase references, now that I have a more complete picture of the situation I agree this is the better solution. And the Tracking of objects looks gorgeous, that should really help when sending data over network!

PS: I've been quite inactive for a while because my job just got a whole lot busier, which probably stays true for the coming weeks as well... I'll see what I can do in between working hours

No worries. You made my job much easier by at least extrapolating all the properties the GUI needed. So please don't feel like your efforts were in vein.

[drdev]

Since I'm already working on large changes to the game data structure to support tracking changes for network play and improving performance, I felt it was a good time to start working on refactoring how keywords are handled since I'm fairly confident that offers a large opportunity to improve performance.

I've created a new Keyword enum that defines a value for every keyword in Magic. Each takes 3 parameters: type, allowMultiple, and reminderText. type is a Class<T> where T derives from a new KeywordInstance class. This class will contain information unique to each instance of a keyword that can appear on a card or player, such as amount, cost, or type. allowMultiple is a boolean that determines whether multiple instances of the same keyword can appear on the same card or player or whether multiple are redundant (such as Flying). reminderText is a String for the reminder text associated with the keyword, with %s and %d placeholders to be swapped out for values in each instance using the instance.formatReminderText function.

I then created a KeywordCollection class which wraps a EnumMap<Keyword, List<KeywordInstance>> object. It will expose a contains(Keyword keyword) function that will allow very quickly determining if a keyword exists using O(1) enum lookup rather than having to loop and do String comparisons for each keyword. This should prove much much more efficient for looking for keywords.

This collection class will also support an add(String keyword) function will support parsing out the current String version of the keyword into the proper Keyword and KeywordInstance objects and adding them to the List<KeywordInstance> mapped to the Keyword. This should make transitioning from our current structure to this new structure relatively easy.

Let me know if you have any questions or concerns about this implementation.

Thanks.
-Dan

[friarsol]

AllowMultiple seems like a poor name, since it implies that an effect granting an additional keyword fails (which isn't true, the keyword is still there, multiple versions of the same keyword just don't do anything extra). Maybe "stacks", or "stacking" would be better way to describe it?

[Marek14]

AllowMultiple seems like a poor name, since it implies that an effect granting an additional keyword fails (which isn't true, the keyword is still there, multiple versions of the same keyword just don't do anything extra). Maybe "stacks", or "stacking" would be better way to describe it?

Also: does, for example, protection stack? Multiple instances of the same protection work like one, except when preventing damage when each has its own effect, but that hardly ever does anything meaningful...

[drdev]

Protection does stack, since as I've implemented it, each protection is a separate KeywordInstance. So a card with Protection from red and black will have keywords.get(Keyword.Protection) = List<KeywordInstance> { Protection from red, Protection from black } essentially. A duplicate in there won't really cause any harm.

So what exactly is the use case for retaining multiple copies of Flying in the collection? I was planning to use the allowMultiple field to avoid adding a second KeywordInstance for Flying.

[friarsol]

So what exactly is the use case for retaining multiple copies of Flying in the collection? I was planning to use the allowMultiple field to avoid adding a second KeywordInstance for Flying.

No I meant the verbiage sounds wrong. "combineRepeats" or "stackMultiple" sound like they just merge duplicated (meaningless) keywords, where "allowMultiple" sounds like it going to prevent you from being effected (at all) by Jump if you already have flying.

[drdev]

So what exactly is the use case for retaining multiple copies of Flying in the collection? I was planning to use the allowMultiple field to avoid adding a second KeywordInstance for Flying.

No I meant the verbiage sounds wrong. "combineRepeats" or "stackMultiple" sound like they just merge duplicated (meaningless) keywords, where "allowMultiple" sounds like it going to prevent you from being effected (at all) by Jump if you already have flying.

Ah, I understand. I'll change it to isMultipleRedundant in that case since that mimics the language used by Gatherer.

[drdev]

So I just committed r27875, which finally swaps in the refactored GameView structure I've been working on for the past several days. This should improve performance and fix a lot of the crashes and stuff not updating on the GUI that people have recently reported.

That said, I'm not completely done with the refactoring yet, as I had to leave many references to game objects in the view classes and overall GUI code for time sake, which I need to get rid of before I can consider tackling network support. But we are inching a bit closer nevertheless.