Forge version 1.5.11

[Agetian]

Alright, sorry for the misfires, here they are.

Uploaded! Thanks! Let me know if you make more!

- Agetian

[Max mtg]

The changes you made to iterator of StorageBase are bad. Regular file system never lists all files recursivelly.

We're not a file system though. The only place we offer subfolder support is for decks, and we want to be sure to load all decks, including those in subfolders.

Nope, we are a file system in this case. Decks added to that storage are immediatelly written to disk.

Do you have an alternate suggestion, instead of just saying it's "bad"? Is there a case where we would use these Storage classes and wouldn't want to iterate through subfolders too?

Is that your way of asking for advice?

Look at the one week old revisions of FDeckChooser, it used to read all folders' contents without those hacks in StorageBase.

I told you not to change anything in core package. Why don't you listen to my words?

[drdev]

Even if they are written to disk via that storage, I still feel it makes sense for the iterator to loop through subfolders. If you still disagree, would you be willing to compromise by copying my iterator() definition into a separate treeIterator() (or something named like that) function, that way there's an option for returning an iterator for both just the current storage and one for the full tree?

Also, while we're on this subject, can you add a relative path property to Deck? I'd like to use it for displaying a new column in the DeckManager. It would be set when loading the Deck from storage, with the path being relative to the starting folder. So if you load from decks/constructed, the relative path would be "" for decks directly in that folder, Standard for decks loaded from say the decks/constructed/Standard folder, and Standard/Aggro for decks loaded from say the decks/constructed/Standard/Aggro folder. Does that make sense?

Anyway, to your point, I'm trying to stay out of core as much as possible, but in this case I felt the functionality should exist in core. I'm not going to implement functionality outside of core if it makes more sense for it to live in core. The previous FDeckChooser logic was messy, and I wouldn't want to have to re-implement it multiple places potentially.

I'm sorry if I didn't have ask nicely for advice in this case. I'm just frustrated with getting ripped to shreds by you every time I commit anything, without even a hint of gratitude for what I'm trying to accomplish most of the time. I'd much prefer to collaborate with you and work out disagreements more respectively and without one of us just reverting something the other did. I'm more than happy to clean up my own messes if you point them out, and I'd like to learn enough that you won't feel the need to protect core from me.

[drdev]

Max, I see now what I missed when thinking my change was acceptable. I apologize for fighting you on it. I think you're right and I just need to request you make changes to Core instead of doing them myself. I'm tired of butting heads with you anyway. Better to work together.

With that in mind, can you please work on a way to support creating, loading, saving, and deleting decks in subfolders reliably?

[Agetian]

r24363: I committed an experimental mini-patch that allows the AI to utilize snow-covered lands that it gets to have in limited modes (Sealed Deck and Booster Draft). Currently it'll correctly add available snow-covered lands of the same color as the deck it's trying to build instead of the appropriate non-snow basic lands.

There is one glitch though that I can't really track: in Booster Draft mode, sometimes the AI will add one or (rarely) two snow-covered lands that do not correspond to its colors. I have no idea where that is coming from - it'll correctly add snow-covered lands of appropriate types and also sometimes randomly throw in a land or two that is off-color (e.g. a two-color R/B deck might end up having an odd Snow-Covered Island). It looks like this does not happen in Sealed Deck mode, only in Booster Draft. Does anybody have any idea why this is happening and how to remedy it?

- Agetian

[drdev]

I'm happy to share that I was finally able to take some time today and skin the scrollbars throughout Forge, as well as ItemManager tables. As a result, with the exception of a few dialogs like the Exception dialog and Open/Save File dialogs, Forge is now completely skinned.

To give you an idea of the new look, here's a screenshot from the new Constructed screen:

Let me know what you think of the new scrollbars. I have full control over how they're rendered now, and can tweak them as needed.

Thanks.
-Dan

[Agetian]

@ drdev: Looks great and consistent! I think the current Forge UI looks pretty much like an actual game (instead of an application) and your efforts in skinning the remaining elements of the interface and improving them played a critical role in it!

In my opinion, now that the UI skinning is pretty much complete, certain deficiencies in color choices for various [older] themes have become more pronounced than they were when those themes were first introduced (I guess, simply because the themes were not designed with the fully skinned UI in mind). For instance, one of the most beautiful themes, Firebloom, is very difficult to use because it uses dark font on dark background (which makes things like card tables or the prompt in the match UI very difficult to read), etc. I guess it might be worth for someone who is working on themes themselves (if there is still anyone, I'm not sure) to update certain colors for certain themes to make them look good with the fully skinned UI in mind.

- Agetian

[Agetian]

Just a little reminder about one of the known regressions that are currently unresolved: cards with effects like Back from the Brink are currently broken due to Max's ongoing work, he mentioned he's going to work on fixing it. Not sure how big of a showstopper this is for the upcoming release (are there many cards broken this way?) and how long it might take to get it to work properly again, but is it worth to add this to the known issues list until it's resolved if the new beta is going to be released before the fix is implemented?

- Agetian

[Max mtg]

Added a dedicated input for Convoke, removed gui-calls from ManaCostBeingPaid
Also: removed ItemPoolView class, as well as that class' member - list stored internally (that was used only for sorts performed by ItemManager)

drdev, you have a lot of energy and passion to implement new features, but sometimes that lead you to wrong design desicions, like r24362 and r24349. Not to speak of 24327

I am working on DeckProxy class to cache decks' characteristics useful for deckmanager (including path and those calculated ones, like color and valid formats) and to hold methods to save/delete that deck to/from storage

[drdev]

Added a dedicated input for Convoke, removed gui-calls from ManaCostBeingPaid
Also: removed ItemPoolView class, as well as that class' member - list stored internally (that was used only for sorts performed by ItemManager)

drdev, you have a lot of energy and passion to implement new features, but sometimes that lead you to wrong design desicions, like r24362 and r24349. Not to speak of 24327

I am working on DeckProxy class to cache decks' characteristics useful for deckmanager (including path and those calculated ones, like color and valid formats) and to hold methods to save/delete that deck to/from storage

First of all, thank you for taking on the DeckProxy refactoring. I'm glad to have somebody who understands that code better working on it instead of me trying to figure it out. Do you think you can be done in the next few days so decks in subfolders show up in DeckManagers before the next release?

To your other point, I do my best to make reasonable design decisions quickly so I can accomplish as much as possible with the time I have to work on this project. I don't let the code get in the way of creating new features, rather I try to utilize what exists and refactor/extend as necessary to make new things possible.

I understand you have stricter standards than I do for code organization, which is fine. I don't mind you refactoring what I do or providing alternative implementations for me to do. All I ask is you not break features I've added in the process, and that you maybe be more specific in your criticism so it's more constructive and less antagonistic. That would make it easier for me to learn from what you're saying rather than feeling the need to defend myself.

I really would like to work with you rather than against you on Forge. You're a dedicated developer and an asset to the project, just as I aim to be. I'm also right there with you on big picture ideas for Forge like network support and mobile support. We just need to work out these communication kinks.

Thanks.
-Dan

[Agetian]

Here's the latest summary of the known bugs and regressions (including latest reports in the SVN thread) that might be good to either resolve prior to 1.5.11 if possible or add to the known issues list:

- Card catalog display preferences are currently broken [Dan (drdev) is aware of this issue and had previously said he'd work on reimplementing those preferences differently; ETA currently unknown; if this can't be resolved before 1.5.11, probably worth mentioning in the known issues list and/or temporarily disabling the checkboxes for those preferences in order to make it clear to the players that they don't work temporarily].
- Cost payment for cards similar to Back from the Brink is currently broken [Max (Max Mtg) is aware of this issue and had previously said he'd work on reimplementing the relevant cost payment differently; ETA currently unknown; only affects 1 card according to Max].

- Agetian

[Max mtg]

back from the brink is the only card that used its special cost 'exile and pay' - so no other cards are affected
ETA is really unknown because I've lost svn history during module separation (would love to restore it by reverting and commiting the game module again), then DeckProxy for drdev, then that one.

[Agetian]

r24388: I changed the way basic lands are generated in limited modes again (had to reduce the number of total lands generated since 50 per art was unreasonably too much and made it very hard to sideboard with pretty much a thousand of different basic lands in the sideboard). For now, reduced it to 10 per art if there's more than one picture for a card and to a fixed amount of 30 if there is only one picture for a card. Please let me know if this is enough / too little / too much.

@ Max: Thanks for clarifying the status and the severity level of that payment thing! Good luck with the module separation, so far so good!

- Agetian

[Max mtg]

FOR THE NEXT 30-60 minutes PLEASE DO NOT UPDATE/COMMIT TO SVN (because this might result in loss of work)

UPD: Done. The files moved to Game package now keep their history.

[friarsol]

r24388: I changed the way basic lands are generated in limited modes again (had to reduce the number of total lands generated since 50 per art was unreasonably too much and made it very hard to sideboard with pretty much a thousand of different basic lands in the sideboard). For now, reduced it to 10 per art if there's more than one picture for a card and to a fixed amount of 30 if there is only one picture for a card. Please let me know if this is enough / too little / too much.

Yea I've found this seperation very annoying in Quest Mode. Now my quest shop is filled with Basic Lands, making it tough to see what cards are available at a glance, and what non-basics are available without messing seriously with the filters.