Make three strike battle any other strike battle and add an option to steal tires #5307

zikzaksuper · 2025-02-12T17:59:48Z

Description

adds a spinner widget (or rather, repurposes an existing one) and a checkbox

Issues

i made a mistake by not creating a branch until i already coded all of that in a couple of days ago.
even though i have checked all the modifications to see if i had accidentally removed something please check again just in case.

Agreement

By creating a pull request in stk-code, you hereby agree to dual-license your contribution as
GNU General Public License version 3 or any later version and
Mozilla Public License version 2 or any later version.

This includes your previous contribution(s) under the same name of contributor.

Keep the above statement in the pull request comment for agreement.

kimden · 2025-02-12T19:01:56Z

src/modes/three_strikes_battle.cpp

+        lerp_time = float(m_kart_info[id].m_lives-1)/float(UserConfigParams::m_ffa_time_limit);
+        int lerp_lower = floor(lerp_time*gradientLength);
+
+        color = video::SColor(255, lerp(redGradient[lerp_lower+1],redGradient[lerp_lower+2],lerp_time*gradientLength-lerp_lower), lerp(greenGradient[lerp_lower+1],greenGradient[lerp_lower+2],lerp_time*gradientLength-lerp_lower), lerp(blueGradient[lerp_lower+1],blueGradient[lerp_lower+2],lerp_time*gradientLength-lerp_lower));


For readability, please don't use lines that are that long, and use spaces around operators, after commas, etc. (not just here).

Regarding this particular line, it might be better to introduce an auxiliary function to find the color to not retype the same code three times for three gradient arrays, especially because it's used in another place.

kimden · 2025-02-12T19:04:17Z

src/states_screens/track_info_screen.cpp

+
+        m_tire_stealing->setVisible(!UserConfigParams::m_use_ffa_mode);
+        getWidget<LabelWidget>("tire-stealing-text")->setVisible(!UserConfigParams::m_use_ffa_mode);
+		getWidget<LabelWidget>("tire-stealing-text")->setText(_("Steal tires"), false);


Each tab → 4 spaces

kimden · 2025-02-12T19:06:10Z

src/modes/three_strikes_battle.hpp

+    // ------------------------------------------------------------------------
+    const int redGradient[12] = {30, 64, 255, 255, 255, 128, 0, 0, 0, 0, 0, 0};
+    // ------------------------------------------------------------------------
+    const int greenGradient[12] = {0, 0, 0, 128, 255, 255, 255, 128, 255, 128, 0, 0};


These arrays are used in the implementation only, it might be better to move them to .cpp into e.g. anonymous namespace

kimden

Code style is alright now I suppose.

kimden · 2025-02-13T23:38:04Z

src/modes/three_strikes_battle.cpp

+    {
+        int redVal = lerp(redGradient[lerp_index + 1], redGradient[lerp_index + 2], lerp_time);
+        int greenVal = lerp(greenGradient[lerp_index + 1], greenGradient[lerp_index + 2], lerp_time);
+        int blueVal = lerp(blueGradient[lerp_index + 1], blueGradient[lerp_index + 2], lerp_time);


This could be a separate function on an array/vector to invoke three times, but I suppose at this point it's kinda ok already.

kimden · 2025-02-13T23:51:38Z

I tested the changes a bit.

In the original 3 strikes mode, I think it was impossible to get more than 3 lives. Testing the mode without stealing, you can get a spare kart and get more than the initial number.
If the initial number of lives is 1, you get two wheels for the kart :)
Colors feel very dark, and even for 3 lives for some reason 0 is dark green-yellow, 1 is very dark red, 2 is yellow-green, 3 is cyan. My opinion is subjective but this doesn't seem to be an intuitive color scheme.
If you go above the initial amount due to stealing or taking spare karts, your color is unpredictable:
I'm not sure whether community and/or developers approve the general idea. I find it okayish.

zikzaksuper · 2025-02-14T07:18:04Z

im not sure if its just me but the colors feel alright to me, yes, they're darker, but i think it's still quite distinguishable - at least for me, which is why im going to do something about it. should be just a small change to the gradient values.
as for when you go over the initial amount, i actually initially planned to have a limit, but when i tested without it i liked the funky colors, just feels interesting to "discover" a new life count, especially considering that they depend on the starting life count.

as for the tires, i checked it and theres quite some more related bugs i need to fix

and for using the spare tire karts to get over the initial amount of lives i just kinda forgot when i was modifying that code

edit: i looked it up and the funny colors are because of sampling the gradient outside of range, which is apparently not recomended, so i will fix that

kimden

Other than these, the code seems fine. Colors are better, I'm not sure I like the changing colors over max value, but I'll let developers decide.

kimden · 2025-02-15T09:48:39Z

src/karts/controller/spare_tire_ai.cpp

    {
        if (r)
-            r->addMessage(_("You can have at most 3 lives!"), k, 2.0f);
+            r->addMessage(_("You can't go over the starting amount!"), k, 2.0f);


_("You can have at most %d lives!") might be better to not involve many changes for translation, but I'm not exactly sure because it would involve plural forms of the word life. At the very least, "over the starting amount" implies question "amount of what?" for me, though I'm not a native speaker.

Please consult any Transifex expert among the developers before editing.

kimden · 2025-02-15T09:51:00Z

src/modes/three_strikes_battle.cpp

+        lerp_time = float(m_kart_info[id].m_lives-1)/float(UserConfigParams::m_ffa_time_limit);
+        int lerp_lower = floor(lerp_time*gradientLength);
+
+        color = color = calculateColor(lerp_lower % (gradientLength + 2), lerp_time * gradientLength - lerp_lower);


repeated color =

kimden · 2025-02-15T09:58:38Z

src/modes/three_strikes_battle.cpp

+    /* Must be 3 lower than the length of the array so it doesn't 
+    and so that it connects once you get more lives than the starting amount.
+    The first and last items in the array must be the same.*/
+    const int gradientLength = 9;


You don't have to justify why 9 is 12 minus 3 if you just define arrays using it:

const int GRADIENT_LENGTH = 9; const int redGradient[GRADIENT_LENGTH + 3] = {....} ...

Or maybe just using vectors and not bothering about sizes is fine...

kimden · 2025-02-15T10:16:33Z

src/modes/three_strikes_battle.cpp

+        int greenVal = lerp(greenGradient[lerp_index], greenGradient[lerp_index + 1], lerp_time);
+        int blueVal = lerp(blueGradient[lerp_index], blueGradient[lerp_index + 1], lerp_time);
+        return video::SColor(255, redVal, greenVal, blueVal);
+    }


This function is called twice in two different ifs, those ifs seem identical and related only to the color logic, and not to the game mode itself. Pass the current and the maximum number of lives to this function, and let it do all the transformations with lerp_*** inside. Don't forget to put spaces around operators -*/.

lerp is still repeated three times. Please move it into a separate function or lambda, e.g. return video::SColor(255, f(redGradient), f(greenGradient), f(blueGradient)) with

auto f = [=](const int* a) -> int { return lerp(a[lerp_index], a[lerp_index + 1], new_lerp_time); };

kimden · 2025-02-15T14:17:24Z

src/modes/three_strikes_battle.cpp

-        case 0:
-            color = video::SColor(255, 128, 128, 128);
-            break;
+        color = video::SColor(128,128,128,0);


Is this an intentionally semi-transparent yellow, or it should have been video::SColor(255, 128, 128, 128); ?

i think i wrote that piece of code before noticing the alpha is the first parameter

(although that doesnt explain the 0)

kimden · 2025-02-15T14:26:12Z

src/karts/controller/spare_tire_ai.cpp

    {
        if (r)
-            r->addMessage(_("You can have at most 3 lives!"), k, 2.0f);
+            r->addMessage(_("You can have at most %d lives!"), k, 2.0f);


I'm not an expert but I suppose this would need a pluralized form. Wait until someone else's feedback please.

bart added 4 commits February 12, 2025 17:11

Three strikes battle changes and favorites bugfix

c1098c8

changes to three strike battle

908db4d

changes to three strike battle

8f889d4

return accidentaly removed changes

1fbd355

kimden reviewed Feb 12, 2025

View reviewed changes

change according to review

d97cc0d

zikzaksuper requested a review from kimden February 13, 2025 17:26

kimden reviewed Feb 13, 2025

View reviewed changes

CodingJellyfish added T: enhancement C: Race Events labels Feb 14, 2025

bugfixes and lighter gradient

05eed49

zikzaksuper requested a review from kimden February 14, 2025 09:25

kimden reviewed Feb 15, 2025

View reviewed changes

better gradient function and hopefully final changes

ff55392

zikzaksuper requested a review from kimden February 15, 2025 11:44

gradient bugfix

198b595

kimden approved these changes Feb 15, 2025

View reviewed changes

0 life color fix

55a3481

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make three strike battle any other strike battle and add an option to steal tires #5307

Make three strike battle any other strike battle and add an option to steal tires #5307

zikzaksuper commented Feb 12, 2025 •

edited

Loading

kimden Feb 12, 2025

kimden Feb 12, 2025

kimden Feb 12, 2025

kimden left a comment

kimden Feb 13, 2025

kimden commented Feb 13, 2025

zikzaksuper commented Feb 14, 2025 •

edited

Loading

kimden left a comment

kimden Feb 15, 2025

kimden Feb 15, 2025

kimden Feb 15, 2025

kimden Feb 15, 2025

kimden Feb 15, 2025

zikzaksuper Feb 15, 2025 •

edited

Loading

kimden Feb 15, 2025

Make three strike battle any other strike battle and add an option to steal tires #5307

Are you sure you want to change the base?

Make three strike battle any other strike battle and add an option to steal tires #5307

Conversation

zikzaksuper commented Feb 12, 2025 • edited Loading

Description

Issues

Agreement

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimden commented Feb 13, 2025

zikzaksuper commented Feb 14, 2025 • edited Loading

kimden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zikzaksuper Feb 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zikzaksuper commented Feb 12, 2025 •

edited

Loading

zikzaksuper commented Feb 14, 2025 •

edited

Loading

zikzaksuper Feb 15, 2025 •

edited

Loading