Add blaster_BC_buttons #582

NoSloppy · 2023-08-01T07:45:29Z

No description provided.

profezzorn

Just some initial comments.
IF this things were to inherit from the blaster prop, then there will be lots of changes, so I want to get that out of the way before I review in more detail.

profezzorn · 2023-08-13T22:34:10Z

common/saber_base.h

@@ -77,6 +77,7 @@ extern SaberBase* saberbases;
    DEFINE_EFFECT(UNJAM)			\
    DEFINE_EFFECT(PLI_ON)			\
    DEFINE_EFFECT(PLI_OFF)                      \
+    DEFINE_EFFECT(DESTRUCT)                     \


self-destruct is usually an off-type rather than an effect...

It is,

case NEXT_ACTION_BLOW: Off(OFF_BLAST); break;

But having EFFECT_DESTRUCT allows for blade animations.

We might want to add a way for TrInOut to handle EFFECT_DESTRUCT so we can have a transition from the on to the off color.

Hmm. oh, you mean like an "explosion"? Wouldn't that just be the OUT->IN transition, something like

InOutTrL<TrInstant,TrConcat<TrInstant,Blinking<White,Black,75,500>,TrFade<600>>>

That would play with boom.wav.
I'm not sure what you're saying the integration with EFFECT_DESTRUCT should be.

oh well that's not right. It works for Destruct, but also shows if powering off normally.
So what did you have in mind?
I glanced at styles/inout_helper.h, but do not know what should happen here.
Maybe if EFFECT_DESTRUCT happens, we somehow bypass in_tr_.begin(); ?

Or maybe we just have an EFFECT_BOOM that layers in the blade style on top of the InOutTrL?
Actually, my blaster styles don't even use an InOutTrL.
It's just always on not showing anything most of the time.

Added EFFECT_BOOM, tested, works well for Boom, unless you had a different idea?

props/blaster_BC_buttons.h

profezzorn · 2023-08-13T22:35:42Z

props/blaster_BC_buttons.h

+    PropBase::DoMotion(Vec3(0), clear);
+  }
+
+RefPtr<BufferedWavPlayer> wav_player;


variables should generally go at the bottom (and be indented)

I put it above Event2 where I saw it in other props, and
I just added more bools to the spot that blaster.h has them.
So should the RefPtr (and all the other declarations) be moved to the bottom of the file?

Working on it.

profezzorn · 2023-08-14T07:33:31Z

Needs fixes for dedicated power and reload buttons, esp if dual_prop used.
Guidance?

It would be easier to provide guidance if I knew what the problem was...

NoSloppy · 2023-08-24T02:21:12Z

Needs fixes for dedicated power and reload buttons, esp if dual_prop used.
Guidance?

It would be easier to provide guidance if I knew what the problem was...

dual_prop doesn't account for these buttons I think was / is the issue.
How would that work if we already have a "power" button that gets swapped to a button named BUTTON_FIRE?
Would we just add the 2 additional non-remapped buttons something mostly like this?

#ifdef CONFIG_BUTTONS
Button PowerButton(BUTTON_POWER, powerButtonPin, "pow"); 
Button AuxButton(BUTTON_AUX, auxPin, "aux");
Button ReloadButton(BUTTON_RELOAD, aux2Pin, "reload");
Button ????Button (BUTTON_???? , 17, "???"); // using data3 on v2.2 or TX on v3.9 Proffieboards
#endif

props/blaster_BC_buttons.h

profezzorn

dual_prop doesn't account for these buttons I think was / is the issue.

I guess the question is: what do we want it to do?

props/blaster_BC_buttons.h

profezzorn · 2025-01-15T05:39:52Z

common/saber_base.h

@@ -77,6 +77,7 @@ extern SaberBase* saberbases;
    DEFINE_EFFECT(UNJAM)			\
    DEFINE_EFFECT(PLI_ON)			\
    DEFINE_EFFECT(PLI_OFF)                      \
+    DEFINE_EFFECT(DESTRUCT)                     \


We might want to add a way for TrInOut to handle EFFECT_DESTRUCT so we can have a transition from the on to the off color.

props/blaster_BC_buttons.h

profezzorn · 2025-01-15T05:43:08Z

props/blaster_BC_buttons.h

+    return max_shots_ != -1 && shots_fired_ >= max_shots_;
+  }
+
+void Fire() {


Instead of overrinding the whole Fire function, how about splitting Blaster::Fire into smaller functions and then overriding the ones that are actually different from the base.

props/blaster_BC_buttons.h

NoSloppy · 2025-01-21T10:41:09Z

Needs fixes for dedicated power and reload buttons, esp if dual_prop used.
Guidance?

It would be easier to provide guidance if I knew what the problem was...

dual_prop doesn't account for these buttons I think was / is the issue. How would that work if we already have a "power" button that gets swapped to a button named BUTTON_FIRE? Would we just add the 2 additional non-remapped buttons something mostly like this?
#ifdef CONFIG_BUTTONS
Button PowerButton(BUTTON_POWER, powerButtonPin, "pow"); 
Button AuxButton(BUTTON_AUX, auxPin, "aux");
Button ReloadButton(BUTTON_RELOAD, aux2Pin, "reload");
Button ????Button (BUTTON_???? , 17, "???"); // using data3 on v2.2 or TX on v3.9 Proffieboards
#endif

Changes to blaster.h make this easier now I think. Working on it.

I'll just make the RELOAD button (AUX2) have the option to be dedicated POWER and hope no one brings a 4 button blaster :)

NoSloppy · 2025-01-22T23:14:01Z

props/blaster.h

@@ -87,6 +87,8 @@ mdauto		Sound made when switching to AUTO mode
 #ifndef PROPS_BLASTER_H
 #define PROPS_BLASTER_H

+#define PROP_HAS_GETBLASTERMODE


I don't remember what this was for. OLED maybe? Also not sure why I have it in my local copy but it's not in the master.

It's used here:

ProffieOS/ProffieOS.ino

Line 695 in b88b82d

#ifdef PROP_HAS_GETBLASTERMODE

which is in turn used by BlasterModeF

ah yes I remember that. Cool. I guess it just never made it into the blaster prop?

NoSloppy · 2025-01-23T01:21:07Z

Still making changes as I keep testing and finding things. Getting closer.

NoSloppy · 2025-01-23T12:21:00Z

Still making changes as I keep testing and finding things. Getting closer.

Ok Major revamping done. I guess since all but one comment here has been addressed, I'll just overwrite these with the latest versions and go from there for review.

profezzorn · 2025-01-26T07:38:35Z

Needs fixes for dedicated power and reload buttons, esp if dual_prop used.
Guidance?

It would be easier to provide guidance if I knew what the problem was...

dual_prop doesn't account for these buttons I think was / is the issue. How would that work if we already have a "power" button that gets swapped to a button named BUTTON_FIRE? Would we just add the 2 additional non-remapped buttons something mostly like this?
#ifdef CONFIG_BUTTONS
Button PowerButton(BUTTON_POWER, powerButtonPin, "pow"); 
Button AuxButton(BUTTON_AUX, auxPin, "aux");
Button ReloadButton(BUTTON_RELOAD, aux2Pin, "reload");
Button ????Button (BUTTON_???? , 17, "???"); // using data3 on v2.2 or TX on v3.9 Proffieboards
#endif
Changes to blaster.h make this easier now I think. Working on it.

I'll just make the RELOAD button (AUX2) have the option to be dedicated POWER and hope no one brings a 4 button blaster :)

I think we would need to add some code to dual_prop to handle these kind of scenarios.

profezzorn · 2025-01-26T07:24:33Z

props/blaster.h


+#ifdef BLASTER_SHOTS_UNTIL_EMPTY
+  const int max_shots_ = BLASTER_SHOTS_UNTIL_EMPTY;


Constants don't usually go at the bottom with the variables.

it doesn't seem moved...
(It should be near the top of the class.)

it doesn't seem moved... (It should be near the top of the class.)

I thought I moved that because I remember saying to myself "ok I'll just put it back where it was."
Now you're saying top, so I just put it there now, up above the first function.

EDIT - wait. I did move them? I thought so. Irrelevant now, just weird that they were on the bottom again just 20 minutes ago.
73c533c

Ok I found what happened.
Apparently I had duplicated that part, not moved it?
Should be all good now.

profezzorn · 2025-01-26T07:26:27Z

props/blaster_BC_buttons.h

+  uint32_t auto_time_;
+  RefPtr<BufferedWavPlayer> auto_player_;
+  RefPtr<BufferedWavPlayer> wav_player;
+


Extra empty line.

profezzorn · 2025-01-26T07:28:58Z

props/blaster_BC_buttons.h

+  typedef SoundLibraryV2 SoundLibrary;
+};
+
+  void Setup() {


Why is this global, and indented?

Do you mean it's global, so fix the indent, or is it wrong to be global?

I think it should be inside the prop. (and I think you want an "override" on it)

Done, thanks. Not too aware of the startup routine yet.

profezzorn · 2025-01-26T07:36:45Z

props/blaster.h

@@ -87,6 +87,8 @@ mdauto		Sound made when switching to AUTO mode
 #ifndef PROPS_BLASTER_H
 #define PROPS_BLASTER_H

+#define PROP_HAS_GETBLASTERMODE


It's used here:

ProffieOS/ProffieOS.ino

Line 695 in b88b82d

#ifdef PROP_HAS_GETBLASTERMODE

which is in turn used by BlasterModeF

profezzorn · 2025-01-30T08:39:34Z

props/blaster.h

  }

  virtual bool CheckJam(int percent) {
    int random = rand() % 100;
    return random < percent;
  }

-  virtual void Fire() {
+  bool DoJam() {
 #ifdef ENABLE_MOTION


#if defined(ENABLE_MOTION) && defined(BLASTER_JAM_PERCENTAGE)

profezzorn · 2025-01-30T08:40:17Z

props/blaster.h

-        return;
-      }
-    }
+  virtual int GetBulletCount() {


Seems the same as the inherited version, so we don't need it, right?

I don't see anything overriding this.
Did you mean we don't need the virtual?
I think that's correct.

Sorry, I missed what file this was in.
It shows up as new because the diff is dumb. Maybe just move it up to line 172 to make the diff nicer.

profezzorn · 2025-01-30T08:41:49Z

props/blaster.h


+#ifdef BLASTER_SHOTS_UNTIL_EMPTY
+  const int max_shots_ = BLASTER_SHOTS_UNTIL_EMPTY;


it doesn't seem moved...
(It should be near the top of the class.)

NoSloppy added 4 commits February 6, 2023 02:59

add destruct

df02d64

Merge branch 'profezzorn:master' into Add-BC-Blaster

6fe4def

Add files via upload

6e23e0d

ifdef or operand syntax fix

21fa83e

profezzorn reviewed Aug 13, 2023

View reviewed changes

NoSloppy added 2 commits August 20, 2023 00:59

adjust button held timing for on/off

842d2ce

swap mode cycle and reload button action

4e965d1

NoSloppy added 4 commits August 24, 2023 02:54

Delete blaster_BC_buttons.h

bf997f6

Add files via upload

1a3205e

allow derived classes

9a15317

SB_Effect override change to SB_Effect2

54aef0c

profezzorn reviewed Mar 24, 2024

View reviewed changes

props/blaster_BC_buttons.h Outdated Show resolved Hide resolved

NoSloppy added 2 commits March 24, 2024 00:54

Update SB_Effect to use EffectLocation

3325b63

PVLOG syntax adjust

3b225b5

profezzorn reviewed Jan 15, 2025

View reviewed changes

NoSloppy added 4 commits January 22, 2025 03:41

add IsEmpty

171c981

no virtual needed

b7db818

full overhaul

4245a37

restructured Fire() with helpers

2a8b0ff

NoSloppy commented Jan 22, 2025

View reviewed changes

NoSloppy added 2 commits January 22, 2025 18:16

oops. updated with old file. now correct.

11f2d4e

Merge branch 'master' into Add-BC-Blaster

7fb9a40

NoSloppy added 3 commits January 23, 2025 07:58

Split into helper functions

a69751b

Revamp from 2 years ago PR

aa0780d

line returns

c114f04

exit -> exiting

15bd253

profezzorn reviewed Jan 26, 2025

View reviewed changes

NoSloppy added 6 commits January 26, 2025 04:37

rm newline

09536ae

move consts

73c533c

move Setup() and override

b477cae

Add EFFECT_BOOM

8fc2962

add EFFECT_BOOM to hybrid_font

91e2096

add BOOM printout

1ac807c

profezzorn reviewed Jan 30, 2025

View reviewed changes

NoSloppy added 4 commits January 30, 2025 04:47

move const initialization

06306c1

condense preprocessor dirs

a79d3a0

remove dupe

e60a4e0

move int GetBulletCount()

397a6ad


		#ifdef BLASTER_SHOTS_UNTIL_EMPTY
		const int max_shots_ = BLASTER_SHOTS_UNTIL_EMPTY;

Add blaster_BC_buttons #582

Are you sure you want to change the base?

Add blaster_BC_buttons #582

Conversation

NoSloppy commented Aug 1, 2023 • edited Loading

profezzorn 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

NoSloppy Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoSloppy Jan 26, 2025 • edited Loading

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

Choose a reason for hiding this comment

profezzorn commented Aug 14, 2023

NoSloppy commented Aug 24, 2023 • edited Loading

profezzorn 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

NoSloppy commented Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoSloppy commented Jan 23, 2025

NoSloppy commented Jan 23, 2025

profezzorn commented Jan 26, 2025

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

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

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

NoSloppy commented Aug 1, 2023 •

edited

Loading

NoSloppy Jan 21, 2025 •

edited

Loading

NoSloppy Jan 26, 2025 •

edited

Loading

NoSloppy commented Aug 24, 2023 •

edited

Loading

NoSloppy commented Jan 21, 2025 •

edited

Loading