Refactoring requests

[Max mtg]

I've started this topic to discuss and plan what needs to be changed about the code to make it more versatile, flexible, better, harder, faster and stronger.

Whenever anything needs to be changed in classes used by many other systems, please post your requests and suggestions here.


My first request and objective for now is to remove all references to forge.gui.* and swing from classes located in forge.game.* packages. The reason is quite simple: games should run without GUI to be able pass automated tests, simulate AI vs AI matches (will get handy for tournaments played with bots). So all classes from the game module are expected interact to with players via interface provided by PlayerController.

[ptx]

As I've also posted in the testing topic, reworking the current PlayerController system so that there is a clear(er) separation between :

• Making choices -> this is a job for the ui (whether it be gui, ai, network, test, ...)
• Enumerating options to choose from -> this is somewhat tricky, the logic for doing this should definitely live in the rules engine (so it is not duplicated etc), but in some cases it might make more sense to have the ui code invoke the code, instead of having the rules engine prepare and pass the options to the ui
• Validating choices -> this is obviously a job for the rules engine (though the ui may still want to also invoke the rules engine to do this for presentation or prediction reasons, but that should merely be something extra, the rules engine should never rely on the ui doing this)
• Modifying the game state to implement the choice -> this is purely the rules engine's job, should not be accessible from ui

So PlayerController implementations should never directly modify the Game object (or the combat object etc). Instead they should receive a read-only version (or at the very least treat the normal version as read-only), as well as an object representing the choice to be made. They can then modify this latter object (or return a new object) to specify the action to be taken, so that after the method call has finished, the rules engine can validate that object, and make the required changes to the game state.
So to cast a spell, the PlayerController no longer handles paying costs and putting on the stack etc itself, instead it returns an object representing how costs should be payed, and the rules engine validates that and handles the rest.

[Max mtg]

ptx, this is absolutelly correct, but involves a very large amount of code to handle. So, do proceed with refactoring if you feel confident about it.

[drdev]

ptx, this is absolutelly correct, but involves a very large amount of code to handle. So, do proceed with refactoring if you feel confident about it.

Can we hold off any more major refactoring until after tomorrow's release?

[Max mtg]

ptx, this is absolutelly correct, but involves a very large amount of code to handle. So, do proceed with refactoring if you feel confident about it.

Can we hold off any more major refactoring until after tomorrow's release?

That's just impossible to handle such a huge amount of refactoring in under 1 day.