ho-dev / HattrickOrganizer

Assistant for Hattrick online football manager
https://ho-dev.github.io/HattrickOrganizer/
GNU Lesser General Public License v3.0
195 stars 79 forks source link

[FEATURE] support of man-marking order #660

Closed wsbrenk closed 4 years ago

wsbrenk commented 4 years ago

manmarkingOrder

Currently teamanalyzers team view is my favorite. a 4th button in SubstitutionOverview could start the team analyzer and some drag+drop action could create the man marking match order. But i don't have any idea how to implement that - up to now.

would be great if you could help me.

Originally posted by @wsbrenk in https://github.com/akasolace/HO/issues/632#issuecomment-668243486

akasolace commented 4 years ago

@wsbrenk I haven't followed everything regarding man marking order and it is a functionality I only used once or twice in game. However, do we really need a new GUI for this? can't we not simply list the players in opponent team like what HT is doing? Relying on last position might be misleading, no?

wsbrenk commented 4 years ago

@akasolace it is not as easy as it looks like. in lineup panel, there is no access to opponents information until now. the idea is to use the existing team analyzer view. Editing the lineup (man marking order in substitution view) has to set lineup information in teamanalyzer and the rest of the man marking editing has to happen in team analyzer.

that's it - i think ;-)

akasolace commented 4 years ago

@wsbrenk it sounds very complicated and I would have clearly taken a much simplified approach: I would have done it all in the order tabs, a new man marking order along substitution and replacement .... simply with a dropdown listing opponent players.

in lineup panel, there is no access to opponents information

I think there are 2 ways to deal with it:

  1. at data download we fetch list of players for upcoming opponenent and that information is stored in a hashmap like 5 teamIDs and 5 list of strings
  2. in lineup tab we add a field for opponent :
    • if selected info are populated accordingly (home/away, TS, .....)
    • have possibility to put 'sandbox' to keep existing behaviour (which is lineup simulator in HT)

For sure, I would not have done any GUI .... but it is up to you :-)

wsbrenk commented 4 years ago

@akasolace up to now i really don't know what's the heavier way. as far as i understand you right, we have to double all or nearly all the download stuff that is already done by the team analyzer. the gui part proposed by you is easier to implement, that's right.

akasolace commented 4 years ago

up to now i really don't know what's the heavier way. as far as i understand you right, we have to double all or nearly all the download stuff that is already done by the team analyzer. the gui part proposed by you is easier to implement, that's right

I might be wrong but I think first approach is way easier: we can reuse the parser of players.xml and simply store the list of opponent player last names (similar to HT) ... Your approach is much more work. Also I think performance wise, approach one would be much more fluid, ....

akasolace commented 4 years ago

Also more generally, we can't make assumption that the user will want to mark a player that was aligned in last lineup, I think it is more robust just to list them all.

wsbrenk commented 4 years ago

@akasolace before you can download opponents player, you have to download and select the upcoming opponent teams. as we talked about man marking the very first time, half a year ago, i suggested to integrate the team analyzer to the lineup panel.

if you only want to fetch the playernames of next opponent you could find them in team analyzer table named TA_PLAYERS i think.

wsbrenk commented 4 years ago

Also more generally, we can't make assumption that the user will want to mark a player that was aligned in last lineup, I think it is more robust just to list them all.

that'S of course correct. in team analyzer the user not only can investigate the last lineup of the opponent but all he selected to download. he very easily could see which player sits on the same position each match or if there is a pattern how the positions are swapped.

if there are new players in the team - fresh from the market/transfer list, those players are missed by the team analyzer approach.

wsbrenk commented 4 years ago

since i'm not the gui expert i asked @tychobrailleur if he wants to take over this issue, at least the gui part of it.

tychobrailleur commented 4 years ago

I have started to look into this, and playing around with substitutions (which I have never done TBH, I'm only Supporter Silver), when I create a new substitution in the popup and click OK, the values in the substitutions table are the default values (no player 1, player 2, time or red card status):

Screenshot from 2020-08-22 08-41-41

Edit doesn't seem to work either. Is it just me?

wsbrenk commented 4 years ago

@tychobrailleur i could reproduce this - it is not OK. maybe my man marking changes introduced this bug.

wsbrenk commented 4 years ago

@tychobrailleur i had one cup of coffee time for investigating this. my idea is that the problem is in SubstitutionEditView.getSubstitution. i left one comment there:

// TODO: edit this.substitution instead of creating a new one
public Substitution getSubstitution(int nextOrderID) {

i will go on in the afternoon. now i have a date with my bees. maybe you are ready then.

wsbrenk commented 4 years ago

665 should repair this.

wsbrenk commented 4 years ago

@tychobrailleur need your tutoring. in this commit i tried to use a drawArrow method found in stackoverflow. Although i debugged all lines successfully, no arrow is visible.

Up to now, i have to set the man marking order in Hattrick and download it to the organizer. This commit should display those orders if the opponent player is in the selected lineup of the team analyzers team panel.

if this doesn't fit to your approach, tell me that i should stop working on it.

wsbrenk commented 4 years ago

@akasolace I think there is room for both approaches. Users who know their opponents very well will be happy to be able to enter the order in the combo boxes proposed by you. Users who need to do more research will prefer the team analyzer approach.

Of course this will again increase the work we have to do.

tychobrailleur commented 4 years ago

@tychobrailleur need your tutoring. in this commit i tried to use a drawArrow method found in stackoverflow. Although i debugged all lines successfully, no arrow is visible.

Ok, I can try. I had started to look at something simpler along the lines of what @akasolace was describing (and the HT orders themselves). What commit in particular should I be looking at?

wsbrenk commented 4 years ago

What commit in particular should I be looking at?

this is the change with drawArrow https://github.com/wsbrenk/HO/commit/be0475471a9d133ac47096458070f3b3eb90e95b ( see above: wsbrenk added a commit to wsbrenk/HO that referenced this issue 2 hours ago)

wsbrenk commented 4 years ago

image

after one day of learning JComponent's positions and their parents positions i've got this. it is not nice and does still not work as expected (every time). my last commit is mentioned above.

tychobrailleur commented 4 years ago

I have spent quite a while trying to do something similar with JLayeredPane but haven't made much progress – for some reason I keep getting errors.

I'll try a bit more, but I am still not convinced this approach is the way to go for this feature. I was more envisioning:

Which makes me think: rather than an arrow, maybe simply add a specific colour for each on the view above would be sufficient? I still think however being brought to the Matches tab for this would be confusing?

wsbrenk commented 4 years ago

lineups and opponent analysis fit together wonderfully in my opinion. my suggestion would be to pack them together - not to double their features, no new match panel or things like this. i'm not happy with the idea, drawing a line to edit/display a man marking order. i thought kloppo would have used chalk lines twenty years ago to convey his ideas to his defender.

akasolace commented 4 years ago

As I said before I think we should go for something really simply: a combo box like in HT. A complex GUI interface, as the one proposed, will not bring much in my opinion and will be error-prone and difficult to maintain. I think the support of man-marking is a great addition, let's make it simple and deliver it with 4.0. @wsbrenk how much work would that be to have it finalized with a simple combobox? Maybe you can have this done for 4.0 and we have another ticket specifying "GUI interface for man-marking order" or "merging lineup and opponent analysis". We would mark those tickets with a long dated milestone and we will have time to discuss what we want to do and GUI mockup before implementing. How does that sound ?

wsbrenk commented 4 years ago

@akasolace good idea - i will continue with a simple combo box first.

akasolace commented 4 years ago

@akasolace good idea - i will continue with a simple combo box first.

great, I really think this is the right approach

akasolace commented 4 years ago

@wsbrenk for my info, do you have the icon you want to use for man marking order ? Otherwise you can list your need in #674 ...

wsbrenk commented 4 years ago

@akasolace i found a svg file, but i do not know how to use it. the other icons in order overview are png file icons.

akasolace commented 4 years ago

That is perfect if you already have a svg because as you know we are in the process to convert all png to svg. @tychobrailleur already made lots of commits to convert from png to svg, have you look at code sample?

the other icons in order overview are png file icons.

Everytime you encounter a png file still in use in HO, do not hesitate to list it in #674, this ticket will be a tracker for svg icon request.

wsbrenk commented 4 years ago

image

progress report: this is the man marking edit dialog. the opponent team is selected by team analyzer

akasolace commented 4 years ago

@wsbrenk I really like the combobox !!

akasolace commented 4 years ago

@wsbrenk I see you submitted your PR already so maybe the follow could be an open ticket for discussion for 5.0

the position is the last played position? can you get this one directly from players.xml or do you have to parse a lot of files ? you put age and experience, I don't think it impacts really the man marking effectiveness. I think what could be more useful, is indication of the specialty (green (Tech) and red (Unpred) similar color than for weather penalties/bonus) and maybe either a hoover message or a text somewhere giving info (-10% all skills, boost of 8% because technical, ......)

wsbrenk commented 4 years ago
  • the position is the last played position? can you get this one directly from players.xml or do you have to parse a lot of files ?

yes, and it is from players.xml

wsbrenk commented 4 years ago
  • you put age and experience, I don't think it impacts really the man marking effectiveness. I think what could be more useful, is indication of the specialty (green (Tech) and red (Unpred) similar color than for weather penalties/bonus) and maybe either a hoover message or a text somewhere giving info (-10% all skills, boost of 8% because technical, ......)

i put a lot of stuff into this line which is not viewable at all. i would like a discussion of what information should be displayed in this line and how.

wsbrenk commented 4 years ago

the approach of displaying as arrow in team analyzer is still in place and i would like to go on with that the next time.

akasolace commented 4 years ago

i put a lot of stuff into this line which is not viewable at all. i would like a discussion of what information should be displayed in this line and how.

ok do you want to for option 1 or 2 :

the approach of displaying as arrow in team analyzer is still in place and i would like to go on with that the next time

I am still not sure how you want to represent this without being very misleading ... but happy to discuss it later on ...

wsbrenk commented 4 years ago

i'm a friend of proposal number 1)

there are a lot of things, that can be done further on concerning the man marking.

akasolace commented 4 years ago

should be fixed by #680 => will be closed after tests