Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Music functionality #598

Merged
merged 37 commits into from
May 7, 2017
Merged

Add Music functionality #598

merged 37 commits into from
May 7, 2017

Conversation

fishykins
Copy link
Contributor

This merge will incorporate musical functions for the handling of playback.
-Generic functions for searching and categorizing music, monitoring/initiating playback.
-Standardization of new config entries "Type" and "Tags", allowing for more dynamic music selection.
While this may seem trivial, many mod and mission makers rely on music in their projects. Standardizing how this is done seems the logical step!

EDIT- I took down my last pull request as the branch was incorrectly named. Sorry for the repeat

@jonpas
Copy link
Member

jonpas commented Feb 23, 2017

No one cares about the branch name. :P

@fishykins fishykins changed the title Music Add Music functionality Feb 23, 2017
@fishykins
Copy link
Contributor Author

good to know... thanks!

private _duration = [_className,"duration"] call CBA_fnc_getMusicData;

if (!isNil "_duration") then {
if (_time < _duration) then {
Copy link
Contributor

Choose a reason for hiding this comment

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

you could make this an exitWith instead of a then. That way you could completly remove the _return variable and just return false in line 51.
But readability may suffer if you do this. not sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Style question. Both methods have their downsides. This is fine too.

Choose a reason for hiding this comment

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

Or just use try {} catch {}; :D

none

Returns:
BOOL: true if music was stopped, false if already stopped
Copy link
Contributor

Choose a reason for hiding this comment

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

This return value is useless as it doesn't return false anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Functions that "do stuff" as opposed to setter functions report usually nothing / nil in SQF, but true is ok too.

};
};

for [{_i=0}, {_i < (count _missionConfig)}, {INC(_i)}] do {
Copy link
Contributor

@Dorbedo Dorbedo Mar 9, 2017

Choose a reason for hiding this comment

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

filtering via configProperties could be suitable too
e.g.:

private _allMusic = configProperties [MissionConfigFile >> "CfgMusic", "getNumber(_x>>'duration')>0",true];
_allMusic append configProperties [configFile >> "CfgMusic", "getNumber(_x>>'duration')>0",true];
private _unsortedSongs = [];
{_unsortedSongs pushBackUnique _x;} count _allMusic;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's better by a mile

Function: CBA_fnc_stopMusic

Description:
A function used to stop any music playing. Must have been started with CBA_fnc_playMusic, otherwise it is ignored.
Copy link
Contributor

Choose a reason for hiding this comment

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

the header needs an update due to changed behavior.

none

Returns:
BOOL: true if music was stopped, false if already stopped
Copy link
Contributor

Choose a reason for hiding this comment

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

there is no need to return anymore


playMusic ["",0];
GVAR(track) = nil;
true
Copy link
Contributor

Choose a reason for hiding this comment

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

true can be left out

Copy link
Contributor

@commy2 commy2 Mar 9, 2017

Choose a reason for hiding this comment

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

Disagree. No function should end with an assignment operator. At least return nil. Returning the assignment operator has a bug in SQF.


_a = call {
    _b = 1;
};

>generic error in expression

_a = call {
    _b = 1;
    nil
};

>no error, _a is undefined.


if (!params [["_className","",[""]]]) exitWith {WARNING('No classname was provided');};

private _config = configFile >> 'CfgMusic' >> _className;
Copy link
Contributor

Choose a reason for hiding this comment

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

the order of the config-files has to be switched.
playMusic prefers the missionConfigFile
The same way this function should behave.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. mission should overwrite addon.

};
};

GVARMAIN(compiledMusic) = _unsortedSongs;
Copy link
Contributor

Choose a reason for hiding this comment

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

this value can contain double entries. (same name inside missionconfig and config)
I think, this should not be the case.

//No tags required so add it
_results pushBack _track;
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

; is missing

Function: CBA_fnc_isMusicPlaying

Description:
A function used to return the current time on playing music. Must have been started with CBA_fnc_playMusic
Copy link
Contributor

Choose a reason for hiding this comment

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

This header needs an update ;-)

@fishykins
Copy link
Contributor Author

Opinions please: should the functions be passing around music as config, or as a class name?

@fishykins
Copy link
Contributor Author

I have included CfgMusic, which updates all base arma 3 music. This does two things: firstly it brings all music up BIS's own formatting standards (lots of music was lacking in detail) and secondly it adds "tags", allowing for a broader search criteria.

@fishykins
Copy link
Contributor Author

so now what?

@commy2 commy2 removed the WIP label Mar 15, 2017
@commy2 commy2 added this to the 3.3 milestone Mar 15, 2017
@commy2
Copy link
Contributor

commy2 commented Mar 15, 2017

Now someone must test it I guess. MP and all.


if (isNil QGVARMAIN(compiledMusic)) then {
private _config = configFile >> 'CfgMusic';
private _missionConfig = missionConfigFile >> 'CfgMusic';
Copy link
Contributor

Choose a reason for hiding this comment

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

Theses Values are not used

// CBA_fnc_findMusic
class findMusic
{
description = "Find music of spesified type";
Copy link
Contributor

@Killswitch00 Killswitch00 Mar 18, 2017

Choose a reason for hiding this comment

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

"specific" (or "specified")


Parameters:
1: CLASS or CONFIG- music to play
2: INT- Position to play from (default: 0)
Copy link
Contributor

@Killswitch00 Killswitch00 Mar 18, 2017

Choose a reason for hiding this comment

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

Explain what unit the position is measured in (seconds?)

Parameters:
1: CLASS or CONFIG- music to play
2: INT- Position to play from (default: 0)
3: BOOL- allowed to interupt already playing music (default: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

interrupt

3: BOOL- allowed to interupt already playing music (default: true)

Returns:
BOOL- true if started playing music
Copy link
Contributor

Choose a reason for hiding this comment

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

true if music started playing

@Killswitch00
Copy link
Contributor

What commy2 said - needs MP testing.

@commy2 commy2 modified the milestones: 3.4, 3.3 Apr 15, 2017
@commy2
Copy link
Contributor

commy2 commented Apr 15, 2017

Sorry, I don't want this for the next update, which already had a release candidate. But I'll test and merge this into master as soon as 3.3 is released.

@commy2
Copy link
Contributor

commy2 commented May 1, 2017

Can be merged after v3.3 is released. Don't want to do it now to keep master==release branch.

@commy2 commy2 merged commit 4048d33 into CBATeam:master May 7, 2017
@dedmen
Copy link
Contributor

dedmen commented Apr 20, 2018

This should be extended using the new https://community.bistudio.com/wiki/getMusicPlayedTime command. Could return when the current song is over. or could be used for reliable autoplay in a playlist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants