1

I'm working on a football simulator and i have 9 matches in backgroung on separate threads. And in the method what's in the heart of each thread, there is an event. And when that even occurs (when a goal is "kicked"), I want to update a label (named goalLabel) on the form with the partial result. I wrote a code...:

for (int i = 0; i < 6; i++)
{
    if (i % 2 == 0) homeGoals++;
    else awawyGoals++;
    if (goal != null) goal(this); //(goal is an event)
    Thread.Sleep(1000);
} //this is the full method

...where on each match the exact count of the goals will be 6(and the result will be 3 - 3), so with 9 (9 is fix too) background matches, the goalLabel should change text (6*9=)54 times. However it only changes a few times. And here's the event's eventhandler method:

public void GoalEventHandler(Match match)
{
    string akt = string.Format("{0} {1} - {2} {3}", match.Opps[0].Name, match.Hgoals, match.Agoals, match.Opps[1].Name);
    UpdateGoalLabel(akt);
}

And the UpdateGoalLabel method:

public void UpdateGoalLabel(string update)
{
    if (InvokeRequired)
    {
        MyDel del = new MyDel(UpdateGoalLabel); // yeah, I have a delegate for it: delegate void MyDel(string update);
        Invoke(del, update);
    }
    else
    {
        lock (this) // if this lock isn't here, it works the same way
        {
            this.goalLabel.Text = update;
        }
    }
}

So I can reach and change the label's text, but I dont why it don't changes 54 times. And that would be the goal, to get notified after every single goal.

Any idea?

Thank you in advance.

Update #1: I'm using VS2010.

Here's the code where I launch the threads:

List<Thread> allMatches = new List<Thread>();
foreach (Match match in matches)
{
    Thread newtmatch = new Thread(match.PlayMatch); //this is the first code block I wrote
    allMatches.Add(newtmatch);
    newtmatch.Start();
}

Update #2: Here's where I attach the eventhandlers (this is in the same method, few lines above the previous code block):

matches = new List<Match>();
foreach (Team[] opponents in Program.cm.nextMatches)
{
    Match vmi = new Match(opponents);
    matches.Add(vmi);
    vmi.goal += new Match.goalevent(GoalEventHandler);
}
//Program.cm.nextMatches is a List<Team[]> object that contains the pairs of teams for the next matches;

And I convert those Team arrays to a Match object, because this class has two Team fields, and has the event and the PlayMatch method which is still the method that contains (only) the first code block.

  • Which version C# are you working with VS2005, VS2010, etc? Also, WinForms, WPF? Finally, can you show the code you have that launches all your 9 threads that begin the whole calling process... – DRapp Nov 26 '12 at 03:11
  • I'm using VS2010, and I updated the "question" with the code. Oh, and it's a windows form. – user1852051 Nov 26 '12 at 03:38
  • Where do you attach the event handler? – akatakritos Nov 26 '12 at 03:59
  • A few lines above where I launch the threads. In the same method. View update 2#. – user1852051 Nov 26 '12 at 04:15
  • Update #3: I tried out something: I declared a List of strings, and where the goalLabel's text is updated (so where the code says: this.goalLabel.Text = update; ) I added the following line: stringList.Add(update); //update is obviously equals with the "akt" string what's in the 2nd code block// And after all I wrote out the whole content of this stringList in a messagebox, and I got all the 54 partial results. So WTF is this? There's nothing between the two lines in the code, and the stringList is changing, and being updated, while the goalLabel don't. Question: why?? – user1852051 Nov 26 '12 at 04:26
  • So all 9 threads are updating the same text box on a one second delay? How are you measuring how many updates happen? If you have 9 threads updating the same text box I would guess they text box is updated 9 times in a short time(talking milliseconds or even microseconds) and the last update wins. – taylorjonl Nov 26 '12 at 04:54
  • I see when the label changes text and counting it on my fingers. :D – user1852051 Nov 26 '12 at 10:39
  • Update #4!!! I noticed that every time the goalLabel changes text 6 times. Here's an example of what you see: "Arsenal 2 - 1 Chelsea". And I noticed, that the teams are randomly changing, but the goals is ALWAYS changing the following way: "1 - 0; 1 - 1; 2 - 1; 2 - 2; 3 - 2; 3 - 3;" – user1852051 Nov 26 '12 at 10:42
  • ..continue: It's not quite bad, cause the "test" method should be run this way, and modify the two teams' goalcounters this way... So yeah, it seems they are racing for the label and the background matches' results are updating almost at the same time, and next time that the main thread what controls the UI and the label should accept another change, the results have changed again. – user1852051 Nov 26 '12 at 10:51
  • Really what you need is one label per match. It is sorta like a stack of papers, imagine you had 9 friends that had a piece of paper that needed to be added to a stack of papers. If you are only checking the top paper on the stack and all 9 friends add their paper at the same time(reasonably, e.g. one friend per second), depending on when you sample you will likely only ever see the last paper placed on the stack. – taylorjonl Nov 26 '12 at 16:23
  • I can't have 9 labels... And back to the "friends and papers": yeah, I olny see the last paper, but I will have 54 papers. => now I'm on something that could solve the issue: every time that the label should be updated, I add the "update" string to a queue of strings (that will contains 54 elements). And then a separate method does the updating of the goalLabel with dequeueing the queue every 1 sec. – user1852051 Nov 26 '12 at 21:00

2 Answers2

0

I've had problems with UI refreshes too and have worked with "BackgroundWorker" threading directly instead of just "Thread" class. The BackgroundWorker thread allows you to report progress changed explicitly and call some method outside the thread (ie: the calling UI thread that invoked the secondary thread). So, I've created a custom class derived from the background worker, and also created my own version of the "Match" class you have described

public class Match
{
    public Match( string Home, string Away )
    {
        HomeTeam = Home;
        HomeGoals = 0;

        AwayTeam = Away;
        AwayGoals = 0;
    }

    // simple properties with PROTECTED setters, yet readable by anyone
    public string HomeTeam
    { get; protected set; }

    public int HomeGoals
    { get; protected set; }

    public string AwayTeam
    { get; protected set; }

    public int AwayGoals
    { get; protected set; }

    // just place-holder since I don't know rest of your declarations
    public EventHandler goal;

    public void PlayMatch()
    {
        for (int i = 0; i < 6; i++)
        {
            if (i % 2 == 0) 
                HomeGoals++;
            else 
                AwayGoals++;

            // Report to anyone listening that a score was made...
            if (goal != null) 
                goal(this, null);

            Thread.Sleep(1000);
        } 
    }
}

// Now, the background worker
public class MatchBGW : BackgroundWorker
{
    // each background worker preserves the "Match" instance it is responsible for.
    // this so "ReportProgress" can make IT available for getting values.
    public Match callingMatch
    { get; protected set; }

    // require parameter of the match responsible for handling activity
    public MatchBGW(Match m)
    {
        // preserve the match started by the background worker activity
        callingMatch = m;

        // tell background worker what method to call
        // using lambda expression to cover required delegate parameters 
        // and just call your function ignoring them.
        DoWork += (sender, e) => m.PlayMatch();

        // identify we can report progress  
        WorkerReportsProgress = true;

        // Attach to the match.  When a goal is scored, notify ME (background worker)
        m.goal += GoalScored;
    }

    // this is called from the Match.
    public void GoalScored(object sender, EventArgs e)
    {
        // Now, tell this background worker to notify whoever called IT
        // that something changed.  Can be any percent, just something to
        // trigger whoever called this background worker, so reported percent is 1
        ReportProgress(1);
    }
}

Now, from your calling window that has the label, such as started from a button click...

    private void button1_Click(object sender, RoutedEventArgs e)
    {
        // create your "Matches" between teams
        matches = new List<Match>();
        matches.Add(new Match("A", "B"));
        matches.Add(new Match("C", "D"));
        matches.Add(new Match("E", "F"));

        foreach (Match m in matches)
        {
            // create an instance of background worker and pass in your "match"
            MatchBGW bgw = new MatchBGW(m);
            // tell the background worker that if it is notified to "
            // report progress" to, to pass itself (background worker object)
            // to this class's SomeoneScored method (same UI thread as textbox)
            bgw.ProgressChanged += SomeoneScored;
            // Now, start the background worker and start the next match
            bgw.RunWorkerAsync();
        }
    }

    // This is called from the background worker via "ReportProgress"
    public void SomeoneScored(object sender, ProgressChangedEventArgs e)
    {
        // Just ensuring that the background worker IS that of what was customized
        if (sender is MatchBGW)
        {
            // get whatever "match" associated with the background worker
            Match m = ((MatchBGW)sender).callingMatch;
            // add it's latest score with appropriate home/away team names
            this.txtAllGoals.Text +=
                string.Format("{0} {1} - {2} {3}\r\n", 
                m.HomeTeam, m.HomeGoals, m.AwayGoals, m.AwayTeam );
        }
    }

Yes, it may be more code, but I am explicitly handling who is called back and reporting what on their proper thread... no BeginInvoke required test / actions. Just an alternative to your issue.

DRapp
  • 47,638
  • 12
  • 72
  • 142
  • Thanks, but it's a task at the university, and I need to use Thread. (And I already using beckgroundworker for other tasks.) – user1852051 Nov 26 '12 at 07:30
0

Let me make sure I am understanding what you are doing. You have started 9 threads that loop 6 times, each time updating a text box and sleeping for 1 second. From the sounds of it they are all updating the same label. You commented that if you push the updates to a list you get them all, my guess is that all 9 threads are updating so fast you can't see it and the last one wins.

I am not sure how you are building your UI but I think this would be perfect for WPF and MVVM(Model-View-ViewModel). I wouldn't use WinForms unless you had a damn good reason, WPF is so much easier to work with.

I would create a few view models:

public class MainWindowViewModel : DispatcherObject, INotifyPropertyChanged
{
    public MainWindowViewModel()
    {
        Matches = new ObservableCollection<MatchViewModel>();
    }

    public event PropertyChangedEventHandler PropertyChanged;

    private ObservableCollection<MatchViewModel> _matches;
    public ObservableCollection<MatchViewModel> Matches
    {
        get { return _matches; }
        set
        {
            _matches = value;
            OnPropertyChanged("Matches");
        }
    }

    private void OnPropertyChanged(string propertyName)
    {
        if (PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

public class MatchViewModel : DispatcherObject, INotifyPropertyChanged
{
    public MatchViewModel()
    {
        HomeTeam = new TeamViewModel();
        AwayTeam = new TeamViewModel();
    }

    public event PropertyChangedEventHandler PropertyChanged;

    private TeamViewModel _homeTeam;
    public TeamViewModel HomeTeam
    {
        get { return _homeTeam; }
        set
        {
            _homeTeam = value;
            OnPropertyChanged("HomeTeam");
        }
    }

    private TeamViewModel _awayTeam;
    public TeamViewModel AwayTeam
    {
        get { return _awayTeam; }
        set
        {
            _awayTeam = value;
            OnPropertyChanged("AwayTeam");
        }
    }

    public void PlayMatch()
    {
        for (int i = 0; i < 6; i++)
        {
            if (i % 2 == 0)
                OnGoalScored(HomeTeam);
            else
                OnGoalScored(AwayTeam);
            Thread.Sleep(1000);
        }
    }

    private void OnGoalScored(TeamViewModel team)
    {
        if (!team.Dispatcher.CheckAccess())
        {
            team.Dispatcher.Invoke((Action<TeamViewModel>)OnGoalScored, team);
        }
        else
        {
            team.Score++; // do other stuff here
        }
    }

    private void OnPropertyChanged(string propertyName)
    {
        if (PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

public class TeamViewModel : DispatcherObject, INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    private string _name;
    public string Name
    {
        get { return _name; }
        set
        {
            _name = value;
            OnPropertyChanged("Name");
        }
    }

    private int _score;
    public int Score
    {
        get { return _score; }
        set
        {
            _score = value;
            OnPropertyChanged("Score");
        }
    }

    private void OnPropertyChanged(string propertyName)
    {
        if (PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

Then in the program class on the UI thread, do something like this:

    protected override void OnStartup(StartupEventArgs e)
    {
        base.OnStartup(e);

        MainWindowViewModel mainWindow = new MainWindowViewModel();
        List<Thread> matchThreads = new List<Thread>();
        foreach (Team[] opponents in Program.cm.nextMatches)
        {
            MatchViewModel match = new MatchViewModel();
            match.HomeTeam.Name = opponents[0].Name;
            match.AwayTeam.Name = opponents[1].Name;
            mainWindow.Matches.Add(match);

            Thread matchThread = new Thread(match.PlayMatch);
            matchThreads.Add(matchThread);
            matchThread.Start();
        }

        MainWindow = new MainWindow();
        MainWindow.DataContext = mainWindow;
        MainWindow.Show();
    }

I did mine in the override for OnStartup because in VS2010 when you create a project you would have your starup item inherit from System.Windows.Application.

I have this simple UI for testing:

<Window x:Class="TestMatch.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        Title="MainWindow" Height="350" Width="525">
    <ItemsControl ItemsSource="{Binding Matches}">
        <ItemsControl.ItemTemplate>
            <DataTemplate>
                <TextBlock>
                    <TextBlock.Text>
                        <MultiBinding StringFormat="{}{0} {1} - {2} {3}">
                            <Binding Path="HomeTeam.Name"/>
                            <Binding Path="HomeTeam.Score"/>
                            <Binding Path="AwayTeam.Name"/>
                            <Binding Path="AwayTeam.Score"/>
                        </MultiBinding>
                    </TextBlock.Text>
                </TextBlock>
            </DataTemplate>
        </ItemsControl.ItemTemplate>
    </ItemsControl>
</Window>

This code is pretty crude and thrown together to give a quick demo of a MVVM way of doing this. I was able to get all 9 teams to update every second. I had some filler code to simulate the objects you had:

public partial class Program
{
    protected override void OnStartup(StartupEventArgs e)
    {
        ...
    }

    private class Team
    {
        public string Name { get; set; }
    }

     private static class cm
     {
         static cm()
         {
             nextMatches =
                 new Team[][]
                     {
                         new[] { new Team { Name = "TeamA" }, new Team { Name = "TeamB" }},
                         new[] { new Team { Name = "TeamC" }, new Team { Name = "TeamD" }},
                         new[] { new Team { Name = "TeamE" }, new Team { Name = "TeamF" }},
                         new[] { new Team { Name = "TeamG" }, new Team { Name = "TeamH" }},
                         new[] { new Team { Name = "TeamI" }, new Team { Name = "TeamJ" }},
                         new[] { new Team { Name = "TeamK" }, new Team { Name = "TeamL" }},
                         new[] { new Team { Name = "TeamM" }, new Team { Name = "TeamN" }},
                         new[] { new Team { Name = "TeamO" }, new Team { Name = "TeamP" }},
                         new[] { new Team { Name = "TeamQ" }, new Team { Name = "TeamR" }},
                     };
         }

         public static Team[][] nextMatches { get; set; }
     }
}
taylorjonl
  • 869
  • 9
  • 15
  • I make you sure, you understand what I'm doing. – user1852051 Nov 26 '12 at 08:26
  • Damn good reason? I only learned winforms. :D I'm on my way ATM and I'm trying to understand the code you wrote (tough I don't have a VS in front of me). So you say that winforms is not capable or what to do this and WPF is? Can you (or anybody else) explain it why? I wrote my full program in winforms and I call this match-simulator form from an other winform with ShowDialog(). Can I do this with WPF too? And in this match simulator form I have a lot of other stuff I of course I would like to avoid to write it all again.. So can anybody explain? – user1852051 Nov 26 '12 at 08:57
  • Ahhh... I try to implement your code, but I don't know how to use WPF. – user1852051 Nov 26 '12 at 12:47
  • ..continue: Can you write it down what are you "using" /at the top/. And now I checked the label's textChanged event, and it changes text 54 times, so the event handler runs down 54 times, just the awayGoals and the homeGoals fiels have already incremented... So any other idea how to hold on those threads to not to increment those fields until they haven't showed off on the gaolLabel? – user1852051 Nov 26 '12 at 12:55
  • If this is for a university project I wouldn't learn WPF for it. I am not saying WinForms can't do it, just it is easier to use WPF, IMO. Is there a requirement to have a single label? If so, you need to find a more advanced way to display the scores. One possibility is to increase the sleep to more than a second, then create a timer that loops through the teams, displaying a new score every second or so. – taylorjonl Nov 26 '12 at 16:09
  • Mmm... I'm trying to understand the above code that you wrote, but I even don't know where is a 'label', and how it's updated... You said that you got 54 partail results. Right? How? And why does it work? – user1852051 Nov 26 '12 at 20:49
  • WPF works differently than WinForms, a TextBlock is a light-weight Label. In WPF with MVVM you design a view model that is the state of your view, then you data-bind elements on the view to properties on the view model. Then as the view model(state) changes, the view is automatically updated. Here is a good link for beginners http://www.codeproject.com/Articles/126249/MVVM-Pattern-in-WPF-A-Simple-Tutorial-for-Absolute – taylorjonl Nov 26 '12 at 21:23
  • But if the label or what updates automatically (and I guess immediately), why didn't it updated only 6 times (more precisely: 6 times slowly and in each of them, 9 times very fast)? How had you managed it to update 54 times slowly? – user1852051 Nov 26 '12 at 21:43
  • Because I had 9 "labels", one for each thread. You had 9 threads accessing a single label, so the last thread to update the label overwrote the other threads updates. Remember, in one of your tests you had the code add the update to a list and in the end this list contained all the updates, so you know the code was executed. You are judging failure because your human eyes could only see one of the nine updates, this is because they probably happened within 5 MS of each other. – taylorjonl Nov 26 '12 at 22:30
  • Ok, I slowed down the program with longer sleeps, and every Sleep(x) is generated randomly. So here is the "new" line in the PlayMatch method: "Thread.Sleep(r.Next(r.Next(6, 16) * 1000, r.Next(16, 26) * 1000));" Now I just have one problem: When there is no update (so there's a 10 secs long nothing, or when they are completely done, I want to clear the label's text after 3 secs. Any idea how to do that? – user1852051 Nov 26 '12 at 23:55
  • Ohh, the good old stopwatch solved the problem... Ok, I think I'm done, the program (seems to) run(s) perfectly. Thanks for the help!! Bye! – user1852051 Nov 27 '12 at 00:37