Handle disposable objects propertly

[Marioalexsan]

Description

Currently, SFML.Net has no checks for whenever an object is disposed (i.e. its CPointer is set to IntPtr.Zero), and will call methods and getters without complaining (before promptly crashing).

The main reasons why this is not great are:

1. Users will crash with a potentially unclear reason as to why (such as "Attempting to write into protected memory", i.e. the classic segfault)
2. Debugger sessions (such as VS) are unable to handle SFML's disposed objects correctly and will promptly crash and stop the debugging session upon attempting to view that object's properties in the Locals / Watch tabs.

Solving this issue isn't too hard, but adds checks across the entire codebase:

• Most implementations of ToString() have been changed so that they display that an object has been disposed instead of attempting to read properties for it
• All member function calls and properties that use CPointer / do CSFML calls have been guarded with a method that throws an ObjectDisposedException when trying to access those methods and properties.

How to test?

Run each of the example projects and make sure they don't crash for normal objects (i.e. not disposed.)
Additionally, run code similar to this one in a debugger session to make sure disposed objects can be visualized correctly in Locals / Watch:

var disposables = new IDisposable[]
{
    new Music("orchestral.ogg"),
    new Sound(new SoundBuffer("canary.wav")),
    new SoundBuffer("canary.wav"),
    new SoundBufferRecorder(),

    new Font("DroidSansMono.ttf"),
    new Image("trollhd.png"),
    new RenderTexture(1280, 720),
    new RenderWindow(new VideoMode(1280, 720), "Test"),
    new Shader("vert.vert", null, "frag.frag"),
    new Sprite(new Texture("trollhd.png")),
    new Text("Test", new Font("DroidSansMono.ttf")),
    new Texture(128, 128),
    new View(),
    new SFML.System.Buffer(),
    new Cursor(Cursor.CursorType.Arrow),
    new Window(new VideoMode(1280, 720), "Test2"),
    new WindowBase(new VideoMode(1280, 720), "Test3"),

    new RectangleShape(new Vector2f(128, 128)),
    new CircleShape(128, 128),
    new ConvexShape(32)
};

foreach (var obj in disposables)
{
    obj.Dispose();
    Console.WriteLine(obj.ToString());
}

[DemoXinMC]

This might be a stupid suggestion, but instead of replacing the hundreds of references to CPointer, couldn't we just build these checks into the existing CPointer getter?

[eXpl0it3r]

I asked the same question on Discord, but some of the CPointers are accessed when they're IntPtr.Zero and they shouldn't throw then.

[DemoXinMC]

It seems to me like it would be better to rework these situations rather than changing every single reference to CPointer still.

Would something like a Valid property (name could be a variety of things) be better than checking CPointer == IntPtr.Zero?

[Marioalexsan]

Since CPointer is a public property, I don't think it's a good idea to change it to throw if the value is IntPtr.Zero. There might be user code that does something with that CPointer without caring about its value, and this would be an API break in a way.

[Marioalexsan]

Although now that I think about it, CPointer is supposed to be an internal detail, so we probably don't care much about that issue?

[eXpl0it3r]

Thank you for implementing this! 🙂