Addresses new code reviews

- Addresses code review from PhysSong and Messmerd
IanCaio · Oct 17, 2023 · 38dd8ec · 38dd8ec
1 parent fe42e97
commit 38dd8ec
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 9 deletions.
diff --git a/include/DataFile.h b/include/DataFile.h
@@ -128,7 +128,7 @@ class LMMS_EXPORT DataFile : public QDomDocument
  void upgrade_bbTcoRename();
  void upgrade_sampleAndHold();
  void upgrade_midiCCIndexing();
- void upgrade_noteTypes();
+ void upgrade_noteTypes();
 
  // List of all upgrade methods
  static const std::vector<UpgradeMethod> UPGRADE_METHODS;

diff --git a/include/Note.h b/include/Note.h
@@ -114,10 +114,7 @@ class LMMS_EXPORT Note : public SerializingObject
  Step
  };
 
- Type type() const
- {
- return m_type;
- }
+ Type type() const { return m_type; }
  inline void setType(Type t) { m_type = t; }
 
  // used by GUI

diff --git a/src/core/Note.cpp b/src/core/Note.cpp
@@ -200,6 +200,7 @@ void Note::loadSettings( const QDomElement & _this )
  m_length = _this.attribute( "len" ).toInt();
  m_pos = _this.attribute( "pos" ).toInt();
  // Default m_type value is 0, which corresponds to RegularNote
+ static_assert(0 == static_cast<int>(Type::Regular));
  m_type = static_cast<Type>(_this.attribute("type", "0").toInt());
 
  if( _this.hasChildNodes() )

diff --git a/src/core/TimePos.cpp b/src/core/TimePos.cpp
@@ -161,6 +161,10 @@ tick_t TimePos::getTickWithinBeat( const TimeSig &sig ) const
 
 f_cnt_t TimePos::frames( const float framesPerTick ) const
 {
+ // Before, step notes used to have negative length. This
+ // assert is a safeguard against negative length being
+ // introduced again (now using Note Types instead #5902)
+ assert(m_ticks >= 0);
  return static_cast<f_cnt_t>(m_ticks * framesPerTick);
 }
 

diff --git a/src/gui/clips/MidiClipView.cpp b/src/gui/clips/MidiClipView.cpp
@@ -468,8 +468,7 @@ void MidiClipView::paintEvent( QPaintEvent * )
  QPixmap stepon200;
  QPixmap stepoff;
  QPixmap stepoffl;
- const int steps = std::max(1,
- m_clip->m_steps);
+ const int steps = std::max(1, m_clip->m_steps);
  const int w = width() - 2 * BORDER_WIDTH;
 
  // scale step graphics to fit the beat clip length
@@ -492,7 +491,7 @@ void MidiClipView::paintEvent( QPaintEvent * )
 
  for (int it = 0; it < steps; it++) // go through all the steps in the beat clip
  {
- Note * n = m_clip->noteAtStep(it);
+ Note* n = m_clip->noteAtStep(it);
 
  // figure out x and y coordinates for step graphic
  const int x = BORDER_WIDTH + static_cast<int>(it * w / steps);
@@ -503,7 +502,7 @@ void MidiClipView::paintEvent( QPaintEvent * )
  const int vol = n->getVolume();
  p.drawPixmap(x, y, stepoffl);
  p.drawPixmap(x, y, stepon0);
- p.setOpacity(sqrt( vol / 200.0 ));
+ p.setOpacity(std::sqrt(vol / 200.0));
  p.drawPixmap(x, y, stepon200);
  p.setOpacity(1);
  }