thread-driven changes of input

[moomarc]

Oh, I didn't specify that I meant only cards with X present in mana cost.

I figured that was most likely what was meant, which is why I used Spoils of War as an example of the exception. It was the only one I could find on magiccards.info where the card gives a value for X (so excluding our others that calculate X based on targets). XChoice should probably all be converted to AnnounceX though.

[myk]

Myk, I don't want to access GUI from several threads. When it comes to reading player's input during a match, only EDT (which I invoke by SwingUtilities.invokeLater) works with UI.

If this is the case, then I am relieved. It just didn't sound like this earlier when you spoke of different threads accessing the swing classes, which is never ok, even if all other threads are blocked at the time.

[Max mtg]

It just didn't sound like this earlier when you spoke of different threads accessing the swing classes, which is never ok, even if all other threads are blocked at the time.

Never meant to deal with swing from many threads. I hardly want to deal with that swing of yours
Don't even see a post where I would have written such thing.

[friarsol]

Hey Max,

I know someone is going to complain that they can't insert a number higher than 100 for X Announce. Do you have a better solution than that?

[Max mtg]

String input with further result parsing in calling routine was worse anyway.

Who is that somebody and what are they going to cast with announce over 100 required? Once you give me that example, I'll add an option for 'enter other number' and reduce option list to 20 items.

[friarsol]

Who is that somebody and what are they going to cast with announce over 100 required? Once you give me that example, I'll add an option for 'enter other number' and reduce option list to 20 items.

Anyone who wants X to be a bigger number? We have a Quest AI deck who gains a ton of life. If they get over 100 life, and if X direct damage spells start using Announce, the Human wouldn't actually have a way to deal a 200 damage with their Blaze if they had a high mana generation deck.

[Max mtg]

Done.

Oh, and here I have question for you, Sol:
- Why have you decided to store the selected value in such a crooked way?

Code: Select all

                 this.setSVar(aVar, "Number$" + value);

The one who is to use this value has to getSVar, split it on dollar sign, then parseInt the second value. Though it could be much simplier if you directly stored a number.
Why haven't you just added a field to SA class?

[friarsol]

Done.

Oh, and here I have question for you, Sol:
- Why have you decided to store the selected value in such a crooked way?

Code: Select all

                 this.setSVar(aVar, "Number$" + value);

The one who is to use this value has to getSVar, split it on dollar sign, then parseInt the second value. Though it could be much simplier if you directly stored a number.
Why haven't you just added a field to SA class?

Because it's consistent with how X is already looking for the value. When X is defined it gets set to an X SVar. When you choose X, you set it to the same X SVar. Generally, this value is going to be used down the line in the Ability resolution, so we set it to an SVar to be accessed later.

I'm confused how "TargetSelection" is a worse name then "TargetChooser" it's very frustrating to try to work on code when the names of the classes are changing every few days.

[Max mtg]

That has to be changed.
If it were not these svar-s, we could store all data in fields that have their type strictly defined and can be searched for usages. Data of uncertain type and referenced by string keys is very hard to manage.

I am open to discussion of this problem.
The name "Target Selection" might refer to both the selection process and the set of selected targets. Meanwhile the purpose of this class is 'show UI to let human choose targets', and besides it also holds one static method referenced from some other classes.
I renamed the class with the pure aim to avoid confusion with TargetChoice. Feel free to give it a different name not to be mixed up with anything else.
And the mentioned method is to be moved out of there.

[friarsol]

That has to be changed.
If it were not these svar-s, we could store all data in fields that have their type strictly defined and can be searched for usages. Data of uncertain type and referenced by string keys is very hard to manage.

I am open to discussion of this problem.

That's definitely outside the scope of thread driven input changes. Let's hold off on this for now. If this is going to happen, we need to synchronize naming conventions between Ability Scripts first.

I am open to discussion of this problem.
The name "Target Selection" might refer to both the selection process and the set of selected targets. Meanwhile the purpose of this class is 'show UI to let human choose targets', and besides it also holds one static method referenced from some other classes.
I renamed the class with the pure aim to avoid confusion with TargetChoice. Feel free to give it a different name not to be mixed up with anything else.
And the mentioned method is to be moved out of there.

I don't want to give it a different name. Target Selection, as in the Selection of Targets. Not Selected Targets. There wasn't any confusion. It's frustrating because I said in the New Features thread I was going to be working on Choose new target cards (like Swerve) and you just changed the names of files I was working on out from under me. I seem to ask you for notice and discussion for this type of thing often.

[Max mtg]

> out of scope of this thread
Nevermind, the core is deployed already... not much of the original subject left to discuss.

I don't believe already that names will be synchronized ever.


There was a confusion. Selection can be understood as both a process and a set of items. There should be crystal clarity about role of each class to save developers' time. That helper class aimed to choose targets via GUI is no kind of "selection".

Sol, with the latest commits I have improved the codebase for you, see r20702 for instance. I have even renamed the class back after cleaning it of methods not related to target selection from GUI.
Find the moved methods in SpellAbility. Now it contains methods to check if one ability can target another one.

[friarsol]

There was a confusion. Selection can be understood as both a process and a set of items. There should be crystal clarity about role of each class to save developers' time. That helper class aimed to choose targets via GUI is no kind of "selection".

From Merriam Webster dictionary "Selection : the act or process of selecting" that seems pretty clear to me.

[Max mtg]

Sol, right from there

1 : the act or process of selecting : the state of being selected
2 : one that is selected : choice; also : a collection of selected things

TargetSelection and no other hints from our side, so one cannot decide which meaning is used until he looks though the code.

[Rob Cashwalker]

'show UI to let human choose targets'

Suggest uiSelectTarget or uiChooseTarget (with corrolary: aiSelectTarget or aiChooseTarget).