Split action bar

[ThibNach]

Add Split Action Bar class to automaticaly display the back action in the left of the bar if there is a back action registered and the others actions registered on the right of the bar

---

This change is 

[TheEmidee]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 17 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, @ThibNach, and @VectorGamez)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 7 at r1 (raw file):

Previously, ThibNach (Thibaut Spreux) wrote…

Done

nope

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 17 at r2 (raw file):

bool bSplitActionBarIgnoreOptOut = false;
static FAutoConsoleVariableRef CVarSplitActionBarIgnoreOptOut(

surround with #if !UE_BUILD_SHIPPING

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 54 at r2 (raw file):

bool UGBFSplitCommonBoundActionBar::IsEntryClassValid( TSubclassOf< UUserWidget > in_entry_class ) const
{
    if ( in_entry_class )

!= nullptr

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 92 at r2 (raw file):

}

void UGBFSplitCommonBoundActionBar::SynchronizeProperties()

can be removed

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 151 at r2 (raw file):

    Super::ValidateCompiledDefaults( compile_log );

    if ( !ActionButtonClass )

!= nullptr

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 189 at r2 (raw file):

    for ( const auto * local_player : sorted_players )
    {
        if ( local_player == owning_local_player || !bDisplayOwningPlayerActionsOnly )

invert all the conditions here and below to reduce nesting

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 303 at r2 (raw file):

                            }

                            const bool is_valid_action_a = legacy_data_a || input_action_a;

auto

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 308 at r2 (raw file):

                            if ( ensureMsgf( ( is_valid_action_a && is_valid_action_b ), TEXT( "Action bindings not displayed yet -- array filter enforces they are not included" ) ) )
                            {
                                const bool a_is_back = is_key_back_action( legacy_data_a, input_action_a );

auto

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 316 at r2 (raw file):

                                }

                                const int32 nav_bar_priority_a = get_navbar_priority( legacy_data_a, input_action_a );

auto

---

Source/GameBaseFramework/Public/GBFSplitCommonBoundActionBar.h line 59 at r2 (raw file):

    UPROPERTY( EditAnywhere, AdvancedDisplay, Category = Display )
    uint8 bIgnoreDuplicateActions : 1;

this is not used

---

Source/GameBaseFramework/Public/GBFSplitCommonBoundActionBar.h line 65 at r2 (raw file):

    UPROPERTY( BlueprintReadOnly, meta = ( AllowPrivateAccess, BindWidget ) )
    TObjectPtr< UHorizontalBox > LeftHorizontalBox;

Keep it generic. Maybe someone will want to not put actions on the Left or the Right.
I would rename one CancelButtonContainer and the other one ActionButtonsContainer

---

Source/GameBaseFramework/Public/GBFSplitCommonBoundActionBar.h line 65 at r2 (raw file):

    UPROPERTY( BlueprintReadOnly, meta = ( AllowPrivateAccess, BindWidget ) )
    TObjectPtr< UHorizontalBox > LeftHorizontalBox;

In the same spirit, some would want to use another container. Expose a TObjectPtr< UPanelWidget > for both

[ThibNach]

Reviewable status: all files reviewed, 17 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, @TheEmidee, and @VectorGamez)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 7 at r1 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

nope

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 17 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

surround with #if !UE_BUILD_SHIPPING

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 54 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

!= nullptr

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 92 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

can be removed

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 151 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

!= nullptr

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 189 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

invert all the conditions here and below to reduce nesting

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 303 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

auto

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 308 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

auto

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 316 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

auto

Done.

---

Source/GameBaseFramework/Public/GBFSplitCommonBoundActionBar.h line 59 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

this is not used

Done.

---

Source/GameBaseFramework/Public/GBFSplitCommonBoundActionBar.h line 65 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

Keep it generic. Maybe someone will want to not put actions on the Left or the Right.
I would rename one CancelButtonContainer and the other one ActionButtonsContainer

Done.

---

Source/GameBaseFramework/Public/GBFSplitCommonBoundActionBar.h line 65 at r2 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

In the same spirit, some would want to use another container. Expose a TObjectPtr< UPanelWidget > for both

Done.

[Frickerson]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, and @TheEmidee)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 7 at r1 (raw file):

Previously, ThibNach (Thibaut Spreux) wrote…

Done.

Missed "OnlineSubsystemUtils.h" ^^

[ThibNach]

Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, @TheEmidee, and @VectorGamez)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 7 at r1 (raw file):

Previously, VectorGamez (Viktor Blomme) wrote…

Missed "OnlineSubsystemUtils.h" ^^

Done.

[Frickerson]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, and @TheEmidee)

[TheEmidee]

Reviewed all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, and @ThibNach)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 17 at r4 (raw file):

bool bSplitActionBarIgnoreOptOut = false;
#if !UE_BUILD_SHIPPING

in fact, surround with #if !( UE_BUILD_SHIPPING || UE_BUILD_TEST )

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 17 at r4 (raw file):

bool bSplitActionBarIgnoreOptOut = false;
#if !UE_BUILD_SHIPPING

empty line above the macro

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 186 at r4 (raw file):

    for ( const auto * local_player : sorted_players )
    {
        const auto * action_router = ULocalPlayer::GetSubsystem< UCommonUIActionRouterBase >( owning_local_player );

I meant:

    for ( const auto * local_player : sorted_players )
    {
        if ( local_player != owning_local_player || bDisplayOwningPlayerActionsOnly )
        {
            continue;
        }

        if ( !IsEntryClassValid( ActionButtonClass ) )
        {
            continue;
        }

        if ( const auto * action_router = ULocalPlayer::GetSubsystem< UCommonUIActionRouterBase >( owning_local_player ) )
        {
        }
    }
 }

[TheEmidee]

Reviewed 1 of 2 files at r3, 1 of 1 files at r4.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, and @ThibNach)

[ThibNach]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Fr0oZzFred, @MaxDaniels, and @TheEmidee)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 17 at r4 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

in fact, surround with #if !( UE_BUILD_SHIPPING || UE_BUILD_TEST )

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 17 at r4 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

empty line above the macro

Done.

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 186 at r4 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

I meant:

    for ( const auto * local_player : sorted_players )
    {
        if ( local_player != owning_local_player || bDisplayOwningPlayerActionsOnly )
        {
            continue;
        }

        if ( !IsEntryClassValid( ActionButtonClass ) )
        {
            continue;
        }

        if ( const auto * action_router = ULocalPlayer::GetSubsystem< UCommonUIActionRouterBase >( owning_local_player ) )
        {
        }
    }
 }

Done.

[TheEmidee]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Fr0oZzFred, @MaxDaniels, and @ThibNach)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 189 at r5 (raw file):

    for ( const auto * local_player : sorted_players )
    {
        if ( local_player != owning_local_player && bDisplayOwningPlayerActionsOnly )

I realize the condition is not correct now.

should be:

const auto is_player_valid = LocalPlayer == OwningLocalPlayer || !bDisplayOwningPlayerActionsOnly;

if ( ! is_player_valid )
{
    continue;
}

[ThibNach]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Fr0oZzFred, @MaxDaniels, and @TheEmidee)

---

Source/GameBaseFramework/Private/GBFSplitCommonBoundActionBar.cpp line 189 at r5 (raw file):

Previously, TheEmidee (Michael Delva) wrote…

I realize the condition is not correct now.

should be:

const auto is_player_valid = LocalPlayer == OwningLocalPlayer || !bDisplayOwningPlayerActionsOnly;

if ( ! is_player_valid )
{
    continue;
}

If it's a question of style, I'll make the change tmrw morning but the condition is correct and works.(... I think... you make me doubt... x) )

[TheEmidee]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Fr0oZzFred and @MaxDaniels)