Protected SFML Methods cannot be consumed by CSFML consumers

[ShadowMarker789]

Consider sf::Audio

////////////////////////////////////////////////////////////
bool Music::onGetData(SoundStream::Chunk& data)
{
    const std::lock_guard lock(m_impl->mutex);

    std::size_t         toFill        = m_impl->samples.size();
    std::uint64_t       currentOffset = m_impl->file.getSampleOffset();
    const std::uint64_t loopEnd       = m_impl->loopSpan.offset + m_impl->loopSpan.length;

    // If the loop end is enabled and imminent, request less data.
    // This will trip an "onLoop()" call from the underlying SoundStream,
    // and we can then take action.
    if (isLooping() && (m_impl->loopSpan.length != 0) && (currentOffset <= loopEnd) && (currentOffset + toFill > loopEnd))
        toFill = static_cast<std::size_t>(loopEnd - currentOffset);

    // Fill the chunk parameters
    data.samples     = m_impl->samples.data();
    data.sampleCount = static_cast<std::size_t>(m_impl->file.read(m_impl->samples.data(), toFill));
    currentOffset += data.sampleCount;

    // Check if we have stopped obtaining samples or reached either the EOF or the loop end point
    return (data.sampleCount != 0) && (currentOffset < m_impl->file.getSampleCount()) &&
           (currentOffset != loopEnd || m_impl->loopSpan.length == 0);
}

This is a protected method, intended to be overriden by user classes for custom behaviour when required.

Consider the usecase of wanting to present and draw the waveform of Audio streamed from disk with sf::Audio

This is impossible with consumers of CSFML or those downstream (e.g.: SFML.Net C# Consumers)

I do not know how it ought to be done, but consumers of CSFML should have a way to consume and override or obtain information from these protected methods.

As of right now, if one wants said waveform in C# they would need to fork SFML, create a new class that overrides onGetData, fork CSFML to bind that functionality, and then bind that into C# via SFML.Net. That's a bit much, ain't it?

[Marioalexsan]

Let's consider all of the protected virtual methods in SFML and how CSFML deals with them:

• sf::Music has onGetData, onSeek and onLoop that it extends from sf::SoundStream, but which cannot be overridden from CSFML
• sf::SoundBufferRecorder has onStart, onProcessSamples and onStop that it extends, but which cannot be overridden from CSFML (note: SFML.Net currently just reimplements SoundBufferRecorder from a SoundRecorder + SoundBuffer)
• sf::SoundRecorder has onStart, onProcessSamples and onStop as abstract methods, which CSFML extends via sfSoundRecorder that accepts callbacks in the creation method
• sf::SoundStream has onGetData, onSeek and onLoop methods; CSFML extends this via sfSoundStream which accepts callbacks for the first two abstract methods, virtual onLoop however is not extensible
• sf::Drawable isn't present in CSFML at all
• sf::RenderWindow has onCreate and onSeek that it extends from sf::Window, but which cannot be modified further from CSFML
• sf::Packet has onSend and onReceive virtual methods that cannot be modified from CSFML
• sf::Window - same as sf::RenderWindow
• sf::WindowBase - same as sf::WindowBase

My thoughts about this are:

• sf::Music could use having overrides for onGetData, onSeek and onLoop
• sf::SoundBufferRecorder could be skipped, since it's mostly an utility that can be reimplemented easily from a sf::SoundRecorder and sf::SoundBuffer like SFML.Net does at the moment
• sf::SoundRecorder is covered
• sf::SoundStream 's onLoop could use an override
• the window classes could be overrideable, but I'm not entirely sure what the use case would be
• sf::Packet could use having overrides for onSend and onReceive

The important part about this is that CSFML knows how to deal with abstract methods, but it doesn't have a solution for virtual methods that already have an implementation available.

We could use something similar to mixins to allow CSFML users to override those methods. For example:

// implementation
typedef bool (*sfMusicOnGetDataOriginal)(sfMusic*, sfChunk*); ///< sf::Music::onGetData callback
typedef bool (*sfMusicOnGetDataMixin)(sfMusicOnGetDataOriginal, sfMusic*, sfChunk*); ///< mixin

CSFML_AUDIO_API void sfMusic_setOnGetData(const sfMusic* music, sfMusicOnGetDataMixin mixin);

// sample usage
bool myCustomOnGetData(sfMusicOnGetDataOriginal original, sfMusic* music, sfChunk* data) {
    // do something before the call
    bool result = original(music, data);
    // do something after the call with data
    return result;
}

sfMusic_setOnGetData(myMusic, myCustomOnGetData); // sets the override
sfMusic_setOnGetData(myMusic, NULL); // restores the original sf::Music implementation

Then CSFML would use the given callback in a wrapper class that extends sf::Music:

bool onGetDataOriginal(sfMusic* music, sfChunk* data) {
    sf::SoundStream::Chunk cppData;
    // copy from data to cppData
    bool result = music->onGetData(&cppData);
    // copy from cppData to data
    return result;
}

bool sfMusic::onGetData(sf::SoundStream::Chunk& data) {
    if (!m_onGetData)
        return sf::Music::onGetData(data);

    sfChunk cData;
    // copy from data to cData
    bool result = m_onGetData(onGetDataOriginal, this, &cData);
    // copy from cData to data
    return result;
}

This code might not be correct or make sense that much, but you get the idea. CSFML users would be allowed to do anything they want in the mixin, including doing stuff before the original method, after the original method, and potentially not call the original method to begin with.