Foil rarity fix patch (completed)

[Agetian]

Ah, well, I'm sorry if my phrasing was somewhat awkward, what I meant to say when I said "knowingly" was just that you mentioned that some of your work may not work well or at all for the newer sets but you sounded like you hoped to get the code integrated immediately despite that (in the original submission, anyway - a lot has changed since then and the submission transformed after all the hard work you put into updating it and I put into formatting and tweaking it to get it approved and integrated upstream). So, all I meant to imply was that if the code is expected to be merged with Forge, it would have to be made to work with all the sets first, that is also what I'm referring to as the "project vision" - Forge is coded in such a way that it tries to support all the sets and all the cards it can, from the beginning of history of Magic (LEA) and till this very day. It's an ambitious goal and not an easy one to follow, but that's what makes Forge.... Forge, I don't know. One of the things, anyway.

Yes, several "return to such-and-such plane" sets have been printed in the new era. For example, there was "Innistrad" first (ISD), and then there was "Shadows over Innistrad" (SOI). WotC likes to return to many of its older planes, and I guess sometimes they can't come up with anything more unique than "Shadows over X" or "Battle for X".

Historically we consider the bugs that break the game (make it crash, make an entire card mechanic not work, make booster generation for multiple sets fail to work in an expected way or crash, etc.) the most severe, and we always try to prioritize fixing those (as long as there's a dev with enough spare time and knowledge to take care of it, which, sadly, is also not always immediately possible). New features are added once they are fleshed out enough to work well and not get in the way of existing features or crash/break the game. Non-critical issues are taken care of as the time permits for the devs that are currently available (sadly, most of us / all of us have real life jobs too, which tend to prevent us from working on Forge as much as many of us would probably want to do otherwise).

Btw, as a significant change that we'll be going through this weekend, we're going to be migrating from SVN to Git, which means that you'll be able to start contributing patches to it immediately, but you'll have to do it in separate branches (in fact, ALL of us will be doing it in separate branches, which will then go through the review/approval procedure and get mainlined into the "master" branch once fleshed out and ready for integration, not breaking anything). This should simplify the submission, review and approval procedure, as well as get rid of the "patch" system which I know you dislike very much (you'll submit your changes in the form of a code branch, which is a live copy of the repository as opposed to the static "patch" which is nothing more than a text file).

However, my original point still stands: please do consider taking some time to improve your coding/formatting style. I might sound a bit paranoid about it (though I'm really not), but just trust me on this one - improving your coding style will go a long way towards simplifying the integration procedure for your code since it'll be a lot easier to read, understand, and modify, it'll also reduce the necessity for us to take care of it for you (and untangle your code) as opposed to actually looking for issues and helping you isolate and resolve them.

- Agetian

[KrazyTheFox]

The are several reasons why I haven't recommended you receive direct commit rights yet.

First, the biggest issue I have with your code is definitely your formatting. Forge is already full of poorly formatted/written code and we've been doing a good job of cleaning that up over time. You're getting better at it, but I don't think you're there yet—it's clear that you're newer to programming and this is not a bad thing, but it is something that needs to be improved. It's already hard enough to read other people's code and poor formatting vastly increases that difficulty. As Agetian mentioned, difficult to read code is more likely to contain bugs, and when reformatting it, it's easy to introduce even more. This also makes the project more difficult for newer developers who might want to join in the future.

As for the "project vision", there isn't one, per se. We want to make Forge a fun experience for as many people as we can with a wide range of customization. In particular, we try not to remove options, as everyone has their own idea of what's fun for them. In general, most of the devs have a list of things they'd like to see in Forge and they usually do fit into the game without much question. Occasionally, there's something that people might object to, and at that point it's prudent to ask the other developers what they think, and, if necessary, ask the community. Overall, we're very open to changes and new things, but we also require that they be thoroughly tested by the developer that's implementing them. This does mean you need to have a decent idea of how Forge works and what kinds of things you need to test when making changes. Unfortunately, there's no guide for this and it's all experience, but we would help you out in learning. At the very least, we do not move quickly, especially when introducing changes that touch core parts of the game. Booster generation does qualify here.

A Forge dev also needs to be able to weigh personal preferences against actual severity. Yes, we understand that you have a strong desire to get the foil rarity and such to a better place, but it is not a critical bug. It does not affect the core gameplay, it does not cause crashes, and it is not glaringly obvious to the detriment of enjoyment. It is something that needs fixing, but it should be taken slow and be tested very well before integration.

This project is a game that lets you play Magic. I don't think it's unreasonable to expect a working knowledge of the current state of the game. I didn't know that you hadn't played in a while and I think other devs did not, either. Please let us know and ask what pitfalls you might expect when changing something like the booster generation. We do want to help you understand and avoid bugs/pitfalls. But we can't help if we don't know. Apologies if I did miss this being said earlier.

Unfortunately, your attitude does come off as rather hostile at times, even if you don't intend it to. Instead of asking (I'm just providing examples, not directly quoting) "why in the world would anyone want this?" or "what is this even doing?" and things along those lines, try rephrasing them to "I don't understand the intention here—is there a particular reason this is like this?" or similar. The former phrases do not express a willingness to understand or learn more about the project. A more diplomatic approach can go a long way.

We're all volunteers for this project. As much as I'm sure many of us would like it to be, this is not our job and we don't get paid for the time we spend on it. In fact, I spend about $30-40 per month supporting the project. Agetian has been amazing in dedicating his time to helping you when he has no obligation to. To demand that he do more thorough testing or work harder to not introduce bugs when he's reformatting your code is exceedingly unfair to him. As a potential Forge developer, it is your responsibility to make every attempt to write and test your code properly, even if it takes longer than you'd like. Not doing so wastes what precious little time that other devs have to work on the things they actually want to work on. Missing a bug, misplacing something, or accidentally committing a file is all understandable (and even expected to some degree), but please don't blame or demand that others test your code for you.

We're in the process of some larger project structure changes. This weekend, hopefully, we'll be moving the project off of subversion and onto Git, which will allow us to make the project more open, as we can have people submit change requests and use branching for bigger features. It enables better code review, as we can comment directly on the code that's being submitted. It lets people contribute to the project without having to use the whitelist that we do now. I have stated to the other devs that I wanted to get this all set up before granting you commit rights, as I think this will help us to better help you. Rather than passing around patches, we can show you where your code has problems and how to fix them (or at least where to look). It is my hope that you'll learn more from this and more quickly join the team officially. It should be noted, however, that after the move to Git, nobody will be allowed to commit directly to the master code base—everyone will be required to have another dev approve their work.

As for not informing you of this sooner, I am sorry. I've been busy working on getting all of this done in addition to my regular programming job and it's taxing at times. I should have let you know that this was the plan and that's on me, not you.

In general, the wiki is correct. Most of the devs have needed only small contributions to be added to the team, but most also have a lot more experience than you appear to. This process will be going away with the move to Git and the wikis will all be rewritten to reflect this.

I know all the above can be difficult to hear, but we do want you to get to the point that we feel comfortable letting you have more direct access to the code base and welcomed as a member of the dev team. I have not listed out the above to complain, but to give you a list of things that I think you need to improve upon. We're all here to help you with this.

Finally, bear in mind that English is not the first language of part of the dev team (and certainly a lot of users). Don't jump to conclusions about someone's attitude when it may be a case of poor phrasing. Native English speakers are liable to mess it up from time to time, too!

[Seravy]

Yes, several "return to such-and-such plane" sets have been printed in the new era. For example, there was "Innistrad" first (ISD), and then there was "Shadows over Innistrad" (SOI). WotC likes to return to many of its older planes, and I guess sometimes they can't come up with anything more unique than "Shadows over X" or "Battle for X".

That's...new. I'm pretty sure back in my days they had a policy to avoid set names that can be confused with another set. Dominaria, Phyrexia, etc for example were all returning worlds used in many sets, but they didn't use the name in the set itself. "Invasion", not "Battle for Dominaria". Oh well. Things WoTC does went downhill in more ways than just this.

As for the coding style...everyone codes in the style they understand most. Your first proposed "reformatted" file was close to unreadable for me. Took me hours to figure out it has problems. This is like asking an English developer to write Chinese comments instead. Maybe after 2 or 3 years of working in java I will find that style easier to read and use, but this isn't something you can willingly change, it either happens naturally or not. Also, this is a really subjective topic, unless you explicitly specify "do not use this type of variable" or "avoid calling that method in that way" or "always name your integer variables with the prefix N_", I will have absolutely no idea what is the preferred "style".
Formatting is fine, I found where it is in the editor and unless I forget to press shift-ctrl-F, it's not a trouble. (I am a forgetful person though. I better say that in advance I guess.)

That GiT thing sounds way better, except it also sounds like a lot more work for the people in charge of approval. We'll see how it works out, for now let's say I'm a bit worried - it might improve things tremendously but might also have the opposite effect. Either way it means I can ignore this "become a developer" process entirely as it's becoming obsolete.

Okay, I admit, there is one thing I'm very bad at. Dealing with people. I probably should have mentioned it but I don't think an introduction saying "hey I'm an antisocial guy who always works alone and knows nothing about java, so give me commit rights, the wiki said you will" would have been a very good idea. It's probably worth saying I had very bad experiences with people "helping" in the Mugen community, so I'm overly sensitive to that. You might even say "paranoid".

"newer to programming" I find this somewhat (actually, very) insulting. I hope you were just expressing yourself poorly here. What I'm new to is java specifically, and having other people work on the same thing. (I thought I made this clear in my introduction, maybe not? Okay, might as well go into more detail. Didn't want to appear as a show off. I have a Technical Informatics diploma from college majoring in System Administrator, but obviously we learned a lot of programming too - minimal java though unfortunately. (not sure if those are the correct English translations though). I also went to a programming high school before that. Furthermore I won a bronze medal on IOI and unsurprisingly to get there I had to place high in the National tournaments. Despite all of that - as I'm bad at dealing with people - I'm living as a NEET anyway.)
A person new to programming would never be able to make patches like these, you need a lot of experience to be able to understand someone else's code and then improve it. That's like the hardest thing there is in programming. Especially on a scale as large as this.

Personally, I believe any bugs that have a permanent detrimental effect on player's save files is the most severe thing there is. Incorrect card ratios in packs all across the board is one of those. You just can't play at all without being affected. I suppose if you don't play Quest mode that's a thing but that is the main (and only) mode of gameplay for anyone interested in the card collecting aspect of the game. The problem with these bugs is, even after fixed, the player's save will still contain the wrong cards. Of course, as it was limited to foils only, it's slightly less bad than if it was all cards and I realize I placed a bit too much priority on it.

Not explaining I'm a returning old player in that introduction thread was a mistake but I did mention it many times in my older posts. There are far more developers than I thought so no surprise not everyone read those posts. I stopped playing around 2007-2008, the last block I was still playing the game was Time Spiral, the Lorwyn prerelease was the last event I appeared at I believe. I used to be a level 1 judge but as I wasn't playing anymore, I didn't renew it and sold my cards soon after. Btw failed the level 2 exam because I'm bad at dealing with people. I scored 94/100 on the test though, but that was way too long ago, that knowledge is forgotten or obsolete now. Rules changed a lot since then, and not for the better. One of the main reasons for stopping the game was getting access to buying cards on the internet. I enjoyed trading and collecting cards a lot, and was proud of my foils - I played full foil decks despite not having much money to spend. That's not easy to do if you have to trade for all the cards, in a country with, like, 100-200 players total. Once all the cards in the world was a click (and a big enough wallet) away, there was no longer a point to it. I especially despise the removal of "damage on stack" and my largest and most long term goal is to have an option to get that rule back - similarly to the mana burn option. I'm aware this means the AI has to be taught all the tricks involved in that system so it's a very long term thing.

What does discourage me the most is not the lack of commit access but the amount of time I expect to spend on this. I'm not the type of person to hold back, if I start doing something, I tend to spend thousands of hours on it and then look at the calendar and "hey, where did the last 5 years go, omg my hair is that grey now?".

PS : English is not my first language either

[Agetian]

I prepared the personal ratings patch for integration to the best of my ability. Provided that you're still interested in getting it upstream, feel free to download this patch, test it in the meantime, and then feel free to propose it as a branch after we migrate to Git

- Agetian

[Seravy]

Will do.

[Agetian]

This patch is now integrated.

- Agetian