Skip to content

Conversation

@flibitijibibo
Copy link
Member

Just running this in PR form to see if CI has any complaints!

Copy link
Member Author

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few review comments for Zeb... a large chunk of this is in the main branch now, it stopped applying cleanly after a couple skips though, will update this in the next version of the changeset.

src/FACT.c Outdated
{
--engine->notification_count;
if (index < engine->notification_count)
memmove(&engine->notifications[index], &engine->notifications[index + 1],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need FAudio_memmove since the SDL configs don't reference the C runtime directly.

src/FACT.c Outdated
if (pEngine->prepared_wavebank_count == pEngine->prepared_wavebanks_capacity)
{
pEngine->prepared_wavebanks_capacity *= 2;
pEngine->prepared_wavebanks_capacity = max(pEngine->prepared_wavebanks_capacity * 2, 8);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same but FAudio_max this time

Elizabeth Figura added 5 commits November 22, 2025 11:55
Notifications are supposed to be generated when they are sent. That is,
registering a notification between preparing a wavebank and calling DoWork()
will still result in the notification being sent.

Therefore we cannot set the notification's time or context when preparing the
wavebank, and must delay until DoWork(). This also reduces the amount of space
we are using.
As described in the previous commit, PREPARED notifications are still sent even
if they're not registered yet.
There's only one soundbank notification, but the isomorphic structure seems a benefit.
Elizabeth Figura and others added 12 commits November 27, 2025 00:16
This essentially rewrites the entire notification logic.

XACT tests in Wine show that notifications are essentially held in a stack. The
most recently registered notification that matches the filter is the one whose
context is used to dispatch it.

Previous registered notifications remain until notifications higher in the stack
are removed via UnRegisterNotification() or expiry [if the flags lack
XACT_FLAG_NOTIFICATION_PERSIST].

XACT_FLAG_NOTIFICATION_PERSIST, meanwhile, affects all notification types, not
just DESTROYED.

Co-Authored-By: Ethan Lee <[email protected]>
… or stopped.

The cue must be in prepared state; it can only be played once.
The wave must be in prepared state; it can only be played once.
This is no longer correct after cf2e05f.
@flibitijibibo flibitijibibo marked this pull request as ready for review November 27, 2025 05:19
Elizabeth Figura added 25 commits November 27, 2025 00:23
Get rid of the playingVariation member, which was never set.
In order to add a new "not yet playing" state, which is not really a fade type.
And while we're at it, rewrite the loop to collapse together some common logic.
And set its state to a new state SOUND_STATE_STOPPED.

For non-interactive cues, GetProperties() is supposed to be able to retrieve cue
properties for the sound and track variations which will play, even before
Play() has been called.

Achieve this by initializing the sound instance earlier, but don't actually play
it yet. Instead we use a new state to signify that the sound is not stopped.
This state exists in complement to the "fading in" and "fading out" states, and
is essentially only used to affect when the FAudio sound is played.
They should be returned that way in GetProperties().

Also, return just the maximum weight from GetProperties(), not the difference. Native does this.
We now always have an active wave as long as there is a playing sound.
Multiply by 1000 instead of dividing. Also, just use integer arithmetic;
there's no reason to use float arithmetic here.
Play-wave events do not have "tracks"; they have "waves". Tracks are something else. Avoid using a confusing term here.
So that we actually start with the first variation, not the second.
hubot pushed a commit to wine-mirror/wine that referenced this pull request Dec 5, 2025
@flibitijibibo flibitijibibo merged commit 00791ac into master Jan 1, 2026
24 checks passed
@flibitijibibo flibitijibibo deleted the zeb branch January 1, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants