Learning Sodium: Moving two paddles in Pong (C#)

[RobertWalter83]

As suggested by Stephen, I'm putting this question in here since there is no dedicated user forum yet.

I'm trying to start learning Sodium by implementing Pong, using the C# implementation. When I try to make my two paddles move based on player input, I run into several problems. Notice that I'm aware that there is a lot of potential to refactor this code, it is work in progress after all. Still, I'm welcoming feedback that illustrates any misuses of Sodium.

If I use Transaction.RunVoid instead of Transaction.Run, moving a paddle works only a few seconds before it will be "stuck". This strikes me as odd.

If I use both paddles simultaneously (let's say by pressing Key.Up and Key.W), both paddles stop moving when I release only one key (let's say Key.Up). This is because no KeyDown event is fired anymore, even though Key.W is still pressed). If I continue by pressing Key.Down (whilst still holding Key.W the entire time), both paddles continue moving (showing me that my Key caching logic is not entirely bogus). I realize this is an issue of System.Windows.Input, but I was wondering if you guys have any idea how to achieve the behavior I want (simultaneous movement of two objects). This would most likely solve the problem that every keydown event initially slowes down exisitng movements for a bit.

public partial class MainWindow : Window
{
        const int dxBoard = 600;
        const int dyBoard = 400;
        const int dxBoardHalf = dxBoard/2;
        const int dyBoardHalf = dyBoard/2;

        private readonly IListener listener;
        private readonly HashSet<Key> mpkey = new HashSet<Key>();
        private static readonly HashSet<Key> mpkeyRelevant = new HashSet<Key> { Key.W, Key.S, Key.Down, Key.Up };

        private event KeyEventHandler AllKeyDown;

        public MainWindow()
        {
            InitializeComponent();
            this.cvs.Width = dxBoard;
            this.cvs.Height = dyBoard;
            Rectangle rect1 = RectanglePlayer();
            Rectangle rect2 = RectanglePlayer();
            this.cvs.Children.Add(rect1);
            this.cvs.Children.Add(rect2);

            listener = Transaction.Run(() =>
            {
                Player player1 = new Player(20);
                Player player2 = new Player(dxBoard - 20);
                //Ball ball = new Ball(dxBoardHalf, dyBoardHalf, 0, 0);

                this.AllKeyDown += player1.MovementHandler(Key.W, Key.S, dyBoardHalf);
                this.AllKeyDown += player2.MovementHandler(Key.Up, Key.Down, dyBoardHalf);

                Canvas.SetLeft(rect1, player1.dx);
                IListener l1 = player1.dy.Listen(dy => Canvas.SetTop(rect1, dy));

                Canvas.SetLeft(rect2, player2.dx);
                IListener l2 = player2.dy.Listen(dy => Canvas.SetTop(rect2, dy));

                return new ImmutableCompositeListener(new [] {l1, l2});
            });
        }

        protected override void OnKeyDown(KeyEventArgs e)
        {
            base.OnKeyDown(e);
            if (IsRelevant(e.Key) && !mpkey.Contains(e.Key))
                mpkey.Add(e.Key);

            foreach (Key key in mpkey)
                OnAllKeyDown(new KeyEventArgs(e.KeyboardDevice, e.InputSource, e.Timestamp, key));
        }

        protected virtual void OnAllKeyDown(KeyEventArgs e)
        {
            KeyEventHandler handler = AllKeyDown;
            if (handler != null)
                handler(this, e);
        }

        private bool IsRelevant(Key key)
        {
            return mpkeyRelevant.Contains(key);
        }

        protected override void OnKeyUp(KeyEventArgs e)
        {
            base.OnKeyUp(e);
            if (mpkey.Contains(e.Key))
                mpkey.Remove(e.Key);
        }

        public void Dispose()
        {
            using (this.listener) { }
        }

        private Rectangle RectanglePlayer()
        {
            return new Rectangle
            {
                Fill = new SolidColorBrush(Colors.White),
                Stroke = new SolidColorBrush(Colors.White),
                StrokeThickness = 1.0,
                Width = 15,
                Height = 50,
                RenderTransform = new TranslateTransform(-7.5, -25)
            };
        }
    }

    internal class Player
    {
        internal readonly double dx;
        internal Cell<int> dy;
        internal StreamSink<int> vy;

        internal Player(int dx)
        {
            this.dx = dx;
            this.dy = new CellLoop<int>();
            this.vy = new StreamSink<int>();
        }

        internal KeyEventHandler MovementHandler(Key keyUp, Key keyDown, int initialPosition)
        {
            KeyEventHandler handler = (obj, args) =>
            {
                Console.WriteLine(args.Key);
                if (args.Key == keyUp)
                    this.vy.Send(-1);
                else if (args.Key == keyDown)
                    this.vy.Send(1);
            };

            this.dy = this.vy.Accum(initialPosition, (v, _dy) => (_dy + 5 * v).Clamp(35, 365));

            return handler;
        }
    }
//... Ball and Game classes
}

[ziriax]

I don't know the answer directly, but I see you are combining mutable state (mpkey) with Sodium FRP. I would not do that, since no transaction is controlling the mpkey at first sight. Try to build-up the keyboard-state (the set of keys being pressed) using FRP, by just sending the key-down and key-up codes so a sink, and building the set using an Accum.

[RobertWalter83]

Thanks for the feedback Ziriax. I tried to realize your idea. I would appreciate if you could review the refactoring. The observable behavior I initially described did not change, still I consider this change an improvement. Notice the new members streamKeys to send keys and the operation to perform (remove or add) and cellMpkey to store the keyboard state (I omitted code that didn't change, but in the meantime, I also added a ball to the game).

public partial class MainWindow : Window { // ... private readonly StreamSink streamKeys = new StreamSink(); private Cell<HashSet> cellMpkey;
// ...

public MainWindow()
{
    InitializeComponent();
    // ... adding components

    listener = Transaction.Run(() =>
    {
        cellMpkey = streamKeys.Accum(new HashSet<Key>(), (keykop, mpkey) =>
        {
            if (keykop.kop == Kop.Remove && mpkey.Contains(keykop.key))
                mpkey.Remove(keykop.key);
            else if (keykop.kop == Kop.Add && !mpkey.Contains(keykop.key))
                mpkey.Add(keykop.key);

            return mpkey;
        });

        // ... players and movement setup
    });
}

protected override void OnKeyDown(KeyEventArgs e)
{
    base.OnKeyDown(e);
    if (IsRelevant(e.Key))
        streamKeys.Send(new Keykop(e.Key, Kop.Add));

    foreach (Key key in cellMpkey.Sample())
        OnAllKeyDown(new KeyEventArgs(e.KeyboardDevice, e.InputSource, e.Timestamp, key));
}

// ... OnAllKeyDown

// ... IsRelevant

protected override void OnKeyUp(KeyEventArgs e)
{
    base.OnKeyUp(e);
    streamKeys.Send(new Keykop(e.Key, Kop.Remove));
}

// ... Dispose

// ... Player, Ball, Game, etc.

///

/// Kind of OPeration ///

enum Kop { Remove, Add }

///

/// KEY with KOP ///

internal struct Keykop { internal readonly Key key; internal readonly Kop kop;

public Keykop(Key key, Kop kop)
{
    this.key = key;
    this.kop = kop;
}

}

[ziriax]

Now you are using Sample outside of a transaction, that is not allowed (personally I try to avoid using it as much as possible).

I will try to look into this in more detail tomorrow.

What you basically have to do:

• make a StreamSink for each external event (keyboard, CompositionTarget.Rendering, etc)
• make a single transaction, and in that transaction make all the CellLoops and Cell that when wired together with the StreamSinks compute immutable output state(s) in the Cells
• listen to the output state(s) and either render it, or update the WPF element properties

[the-real-blackh]

sample outside a transaction is allowed - there's no actual rule against doing that, as long as you know that you could miss changes. An appropriate place to use it would be inside some paint() method where all you care about is painting the current state and you don't care what order that occurs in relative to inputs.

[ziriax]

Maybe the problem still exists since you are still using a mutable HashSet data structure in the accum. You could either use an ImmutableHashSet, or make a [Flags] enum for the keys, and return new flags instead of modifying them. I am not sure this solves your problem, but a Cell's contained data type should always be immutable

[ziriax]

Actually, the Cell's contained type can be mutable, but you should treat it as an immutable type. In your case this means generating a new HashSet instead of modifying the existing one.

[ziriax]

This might be a silly suggestion, but could you first verify you are not dealing with keyboard ghosting? See https://www.microsoft.com/appliedsciences/antighostingexplained.mspx

[ziriax]

Here's a quick example of two moving paddles in WPF and .NET 4.6.1

I did not take the time to introduce constants, and I use a stupid Euler integral. Stephen uses quadratic polynomials and correct integrals :-)

MainWindow.xaml

<Window x:Class="Pong.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        mc:Ignorable="d"
        Title="Sodium Duopaddle">
</Window>

MainWindows.xaml.cs

using System;
using System.Windows;
using System.Windows.Input;
using System.Windows.Media;
using Sodium;

namespace Pong
{
    public partial class MainWindow
    {
        [Flags]
        public enum PlayerKeyboardState
        {
            None = 0,
            MoveUp = 1,
            MoveDown = 2
        }

        private readonly Pen _borderPen = new Pen(Brushes.DimGray, 4);

        private readonly Cell<Point> _cPlayerPos1;
        private readonly Cell<Point> _cPlayerPos2;
        private IDisposable _subscription;

        public MainWindow()
        {
            InitializeComponent();

            WindowStyle = WindowStyle.ToolWindow;
            ResizeMode = ResizeMode.NoResize;
            Background = Brushes.Transparent;
            SizeToContent = SizeToContent.WidthAndHeight;
            Content = new FrameworkElement
            {
                Width = 640,
                Height = 480
            };

            var sKeyEvents = new StreamSink<KeyEventArgs>();
            var sRenderEvents = new StreamSink<RenderingEventArgs>();

            KeyDown += (sender, args) => sKeyEvents.Send(args);
            KeyUp += (sender, args) => sKeyEvents.Send(args);
            CompositionTarget.Rendering += (sender, args) => sRenderEvents.Send((RenderingEventArgs) args);

            var paddleVelocity = new Vector(0, 100);
            var sTicks = sRenderEvents.Collect(TimeSpan.Zero, (re, t) => Tuple.Create(re.RenderingTime - t, re.RenderingTime));
            _cPlayerPos1 = PlayerPos(new Point(10, 220), paddleVelocity, Key.Q, Key.A, sKeyEvents, sTicks);
            _cPlayerPos2 = PlayerPos(new Point(610, 220), paddleVelocity, Key.Up, Key.Down, sKeyEvents, sTicks);

            var sPlayerUpdates1 = Operational.Updates(_cPlayerPos1);
            var sPlayerUpdates2 = Operational.Updates(_cPlayerPos2);
            var sPlayerUpdates = sPlayerUpdates1.OrElse(sPlayerUpdates2);

            _subscription = sPlayerUpdates.Listen(_ => InvalidateVisual());
        }

        protected override void OnClosed(EventArgs e)
        {
            _subscription?.Dispose();
            _subscription = null;
            base.OnClosed(e);
        }

        protected override void OnRender(DrawingContext dc)
        {
            var pos1 = _cPlayerPos1.Sample();
            var pos2 = _cPlayerPos2.Sample();

            dc.DrawRectangle(Brushes.Black, _borderPen, new Rect(2, 2, 640 - 4, 480 - 4));

            dc.DrawRoundedRectangle(Brushes.Red, null, new Rect(pos1, new Size(20, 60)), 5, 5);
            dc.DrawRoundedRectangle(Brushes.LimeGreen, null, new Rect(pos2, new Size(20, 60)), 5, 5);
        }

        public static PlayerKeyboardState Update(PlayerKeyboardState state, Key upKey, Key downKey, KeyEventArgs e)
        {
            var flag =
                e.Key == upKey ? PlayerKeyboardState.MoveUp : 
                e.Key == downKey ? PlayerKeyboardState.MoveDown : 
                0;

            return e.IsDown ? state | flag : state & ~flag;
        }

        public static int KeyStateToDirection(PlayerKeyboardState pks)
        {
            return
                pks == PlayerKeyboardState.MoveUp ? -1 : 
                pks == PlayerKeyboardState.MoveDown ? +1 : 
                0;
        }

        public static Cell<Point> EulerIntegral(Point start, Cell<Vector> cVelocity, Stream<TimeSpan> sTicks)
        {
            var steps = sTicks.Snapshot(cVelocity, (dt, v) => dt.TotalSeconds*v);
            return steps.Accum(start, (dp, p) => p + dp);
        }

        public static Cell<Point> PlayerPos(Point startPos, Vector velocity, Key upKey, Key downKey,
            Stream<KeyEventArgs> sKeyEvents, Stream<TimeSpan> sTicks)
        {
            var cKeyState = sKeyEvents.Accum(PlayerKeyboardState.None, (e, s) => Update(s, upKey, downKey, e));
            var cDirection = cKeyState.Map(KeyStateToDirection);
            var cVelocity = cDirection.Map(d => d*velocity);
            return EulerIntegral(startPos, cVelocity, sTicks);
        }
    }
}

[RobertWalter83]

Thanks Ziriax for all the info. Your solution has the behavior I was striving for. I will review it in detail to try and learn from it. First thing that strikes me is that your solution does not use an explicit transaction at all. I've read in the book that Sodium creates a transaction whenever a value is pushed into a Stream or Cell and subsequent state changes are executed within this transaction, but that it is sort of a best practice to have everything in one big transaction. I've yet to read section 8.3, though, so maybe I will get more info there.

[ziriax]

My bad, I should have used a transaction. But in this particular case, it is not needed because I do not use any CellLoop or StreamLoop. In general, I would use a transaction, but I wrote this code in a hurry.

[RobertWalter83]

I understand. I hope you don't mind me asking another question (and feel free to refer me to study the book more if my questions are too broad in scope!).

I made my way through your implementation and refactored my code accordingly. I wonder about the usage of "Operational.Updates", though. Instead of creating the sPlayerUpdates stream and listen to it to invalidate the visuals, for me it also works to listen "directly" to changes to the players' positions. Instead of invalidating to trigger an OnRender, I kept my old logic where I reset the player visuals like so (notice that I encapsulated the logic of your PlayerPos method in my Player class):

 Player player1 = new Player(new Point(20, dyBoardHalf), Key.W, Key.S, streamKeyEvents, streamTicks);
 Player player2 = new Player(new Point(dxBoard - 20, dyBoardHalf), Key.Up, Key.Down, streamKeyEvents, streamTicks);

 Canvas.SetLeft(rect1, player1.cellPos.Sample().X);
 IListener l1 = player1.cellPos.Listen(point => Canvas.SetTop(rect1, point.Y));

 Canvas.SetLeft(rect2, player2.cellPos.Sample().X);
 IListener l2 = player2.cellPos.Listen(point => Canvas.SetTop(rect2, point.Y));

[ziriax]

I am still reading the book myself ;-) So I am on dangerous ground here, as some of my answers might be wrong. But IMO this is a good way to learn from the feedback...

Your approach looks fine to me. As Stephen corrected me, calling Sample outside a transaction can be done, you just need to realize that you might miss some updates, and that is perfectly ok for rendering.

Note that in a typical game, many entities are dynamic, and listening to individual properties might become difficult.

[jam40jeff]

You should not have to keep a reference to the listener in order to keep it alive (this is true of ListenWeak, but Listen should not work this way). I have verified that this is a bug in the C# and F# versions and will fix it as issue #95.

[RobertWalter83]

Hello, I can verify that, in my Pong example, I can use RunVoid now without losing any references to listeners, so I consider this issue closed.

Please notice that I somewhat finished the Pong game and uploaded it to github. The purpose is on the one side to learn Sodium, and on the other side to give others who want to learn it some examples to start out with. Therefore, I really welcome input from more experienced Sodium users about my approach. I opted eventually for an implementation that is close to the Pong example provided by Elm. I am curious to learn if the way I treat the GameObjects and their immutability is correct. This way, a lot of re-instantiations happen when the game is running. I imagine that this results in problems in bigger scenarios. I have a different implementation that uses Streams and Cells on a more fine grained level (comparable to the code I posted in this very thread earlier), but this seemed to me to get quite unwieldy over time. I'd just love to be able to derive some "Dos and Donts" from my first steps with Sodium and share those among the community, so feedback over at the Pong example is really welcome.