Removed the first recording button from SongEditor as it has no use by steven-jaro · Pull Request #7899 · LMMS/lmms

steven-jaro · 2025-05-20T21:31:25Z

Changes:

Editor.h:

Changed the name of the default value of Editor.cpp constructor from "record_step" to "recordStep".
Added bool "recordAccompany" and set to false to reflect the changes in the parammeters on Editor.cpp.

Song.h:

Removed the "record()" method as it will not be used.
Remove parts of the code that had spaces instead of tabs size 4.

SongEditor.h:

Removed the "record()" method as it will not be used.

Song.cpp:

Removed the "record()" method as it will not be used.
Remove parts of the code that had spaces instead of tabs size 4.

Editor.cpp:

Added bool "recordAccompany" to the constructor.
Renamed "stepRecord" to "recordStep" so that every parammeter name starts with "record*"
Added a lambda functionally for conditional button creation.
Changed the order in which the buttons are created. They were ordered in phases, now it is ordered by button.

PianoRoll.cpp:

Added "(true, true, true)" to the parammeters entry for the Editor constructor to reflect which buttons need to be shown.

SongEditor.cpp (the key change):

Added "(false, true, false)" to the parammeters entry for the Editor constructor to reflect which buttons need to be shown. This is for showing only the recordAccompany button in the song editor.
Removed connection fro "m_recordAction" as it wil not be used here.
Removed checking for "m_recordAction" as it doesn't exist anymore.
Removed the "record()" method as it will not be used.

How to test?:
This PR just removes the button in the Song Editor. So one only has to make sure that the correct buttons are shown on each editor and that they work properly as intended.

All of these was decided because there is no really a use for a button that records without playing the song. So the only audio recording button left is the "RecordAccompany". Also this PR solves the first issue out of 4 in #7786.

src/gui/editors/Editor.cpp

src/gui/editors/SongEditor.cpp

…sary loop

steven-jaro · 2025-05-22T02:59:05Z

I added the changes you asked me.

src/gui/editors/Editor.cpp

steven-jaro · 2025-05-22T12:27:16Z

I modified the logic of the set up for all the buttons so when adding the conditions, the code looks as compact and simplified as much as possible. The code order was:
[Action set up for each button (play, record, recordAccompany, stepRecord, Stop)] ->
[Connection set up for each button (play, record, recordAccompany, stepRecord, Stop)] ->
[Toolbar set up for each button (play, record, recordAccompany, stepRecord, Stop)]

And now I changed it to:
[Complete setup for play (action, connection, toolbar)] ->
[Complete setup for record (action, connection, toolbar)] ->
[Complete setup for recordAccompany (action, connection, toolbar)] ->
[Complete setup for stepRecord (action, connection, toolbar)] ->
[Complete setup for stop (action, connection, toolbar)]

I made this change because otherwise I would have to add 9 "ifs" if I wanted to keep the previous order. So for adding just 3 "ifs" I reordered the logic of the code grouping by button instead of by set up stage. I also changed the order of the bools in the constructor and updated the change for the constructor of the PianoRoll and SongEditor

regulus79 · 2025-05-22T15:53:20Z

Nice! That does look much cleaner

include/Editor.h

steven-jaro · 2025-05-22T20:39:02Z

The error is now fixed.

szeli1

Please could you address my suggestions? Also I did not test this PR.

src/gui/editors/Editor.cpp

include/Editor.h

src/gui/editors/Editor.cpp

steven-jaro · 2025-05-23T20:53:42Z

@szeli1 I will adress all your naming and commenting suggestions as soon as I can.

steven-jaro · 2025-05-25T11:23:15Z

@szeli1 I made the changes you requested but I kept the amount of documentation as I think it is useful. In the future, I will adress your suggestion for refactoring some bad variable names (in another PR).

szeli1

I will approve this, but I did not test. I still don't like how many comments there are.

szeli1 · 2025-05-25T16:23:52Z

I will adress your suggestion for refactoring some bad variable names (in another PR).

Don't bother with it, thanks for working on this issue.

steven-jaro · 2025-05-25T16:31:33Z

I will approve this, but I did not test. I still don't like how many comments there are.

I would like to know the opinion from the other devs. If another devs says it is too much commenting, then I will remove them.

…e implementation

szeli1

Making a lambda was a good Idea, I couldn't find any issues, so I will approve this

JohannesLorenz

I only have 3 style suggestions left, but overall this PR is good and can go into testing phase.

src/gui/editors/Editor.cpp

include/Editor.h

…sary loop

…e implementation

…en-jaro/lmms into feature/recording-stage-one

regulus79 · 2025-12-09T22:25:54Z

The requested whitespace changes by @bratpeki don't seem to appear in the current PR diff.

Given that this PR has 3 approvals and has (I believe) been tested by bratpeki, would it be reasonable to merge it now?

Rossmaxx · 2025-12-27T12:15:10Z

Saw this while looking at old PRs, and the approvals seem decent. Will wait for a day before merge, unless anyone objects.

has (I believe) been tested by bratpeki

So, this PR just nuked one recording button from the Song-Editor, not touching anything else.
I can confirm it's not there for SDL, Jack, PulseAudio and ALSA, which I think is sufficient.

I think so. If not, just tell me @bratpeki

include/Song.h

JohannesLorenz · 2025-12-27T21:29:32Z

unless anyone objects

Let's wait until at least the spacing is fixed ;)

Rossmaxx · 2025-12-28T03:58:23Z

Sure @JohannesLorenz

bratpeki · 2025-12-29T11:20:31Z

src/gui/editors/PatternEditor.cpp


 PatternEditorWindow::PatternEditorWindow(PatternStore* ps) :
-	Editor(false),
+	Editor(false, true, false),


BB Editor doesn't actually record anything, right?

I know it's out of the scope of this PR, I'm asking purely informatively.

This is the only remaining issue I have with the PR.
Very easy to segfault this.

Investigating further.

Segfault backtrace, log.txt

src/gui/editors/SongEditor.cpp

bratpeki · 2025-12-29T11:36:25Z

src/gui/editors/Editor.cpp

-	auto addButton = [this](QAction* action, QString objectName) {
-		m_toolBar->addAction(action);
-		m_toolBar->widgetForAction(action)->setObjectName(objectName);
+	auto createButton = [this](


Awesome code cleanup here!

bratpeki · 2026-01-25T22:02:36Z

Let's wait until at least the spacing is fixed ;)

Recently got write access so I'll resolve this now. @steven-jaro FYI, so if you make any more changes, be sure to pull first.

bratpeki · 2026-02-06T18:36:56Z

Okay, took this on.
First "fix" which is more wording than anything is named here.

steven-jaro added 2 commits May 20, 2025 14:45

Removed the first record button from the Song Editor

5e74e3c

Removed commented lines in SongEditor

6cbd25f

regulus79 requested changes May 21, 2025

View reviewed changes

src/gui/editors/Editor.cpp Outdated Show resolved Hide resolved

src/gui/editors/SongEditor.cpp Outdated Show resolved Hide resolved

src/gui/editors/SongEditor.cpp Show resolved Hide resolved

Fixed regulus request about inverted bool name and removed an unneces…

621e69b

…sary loop

regulus79 reviewed May 22, 2025

View reviewed changes

src/gui/editors/Editor.cpp Outdated Show resolved Hide resolved

Modified editor buttons construction logic

73d5293

steven-jaro requested a review from regulus79 May 22, 2025 12:38

regulus79 reviewed May 22, 2025

View reviewed changes

include/Editor.h Outdated Show resolved Hide resolved

Fixed recordAccompany as false by default

2915379

steven-jaro requested a review from regulus79 May 22, 2025 20:09

regulus79 approved these changes May 22, 2025

View reviewed changes

szeli1 self-requested a review May 23, 2025 07:17

steven-jaro mentioned this pull request May 23, 2025

Sample Track Recording Stage One (by Reflexe, LMMS/lmms continuation) #7786

Draft

szeli1 reviewed May 23, 2025

View reviewed changes

Fixed szeli1 request for consisten documentation

21fece4

steven-jaro requested a review from szeli1 May 25, 2025 11:24

szeli1 approved these changes May 25, 2025

View reviewed changes

Fixed too much comments and implementation of a lambda to simplify th…

b981b53

…e implementation

steven-jaro requested a review from szeli1 May 26, 2025 23:01

szeli1 approved these changes May 28, 2025

View reviewed changes

JohannesLorenz approved these changes May 28, 2025

View reviewed changes

src/gui/editors/Editor.cpp Outdated Show resolved Hide resolved

src/gui/editors/Editor.cpp Show resolved Hide resolved

include/Editor.h Outdated Show resolved Hide resolved

Fixed use of new function for all buttons and step_record to recordStep

036f4b4

steven-jaro added 12 commits November 22, 2025 15:34

Removed commented lines in SongEditor

415dd40

Fixed regulus request about inverted bool name and removed an unneces…

65dfab6

…sary loop

Modified editor buttons construction logic

094d31f

Fixed recordAccompany as false by default

614be09

Fixed szeli1 request for consisten documentation

0c1e2f5

Fixed too much comments and implementation of a lambda to simplify th…

6dd45e0

…e implementation

Fixed use of new function for all buttons and step_record to recordStep

f7e787c

Fixed 4 tab size instead of 4 spaces

b277794

Update Song.h

db68986

Update Song.h

7f8ddb6

Update Song.cpp

e903b5a

fixed spaces into tabs

aafedd7

steven-jaro requested review from bratpeki and szeli1 November 22, 2025 22:19

steven-jaro added 3 commits November 22, 2025 16:35

Fixed conflicts with new changes

62f5365

Merge branch 'feature/recording-stage-one' of https://github.com/stev…

8051313

…en-jaro/lmms into feature/recording-stage-one

Showed record accompany into patterneditor and removed whitespaces

6e02059

steven-jaro requested review from JohannesLorenz and regulus79 November 23, 2025 00:30

Rossmaxx reviewed Dec 27, 2025

View reviewed changes

include/Song.h Show resolved Hide resolved

bratpeki reviewed Dec 29, 2025

View reviewed changes

src/gui/editors/SongEditor.cpp Outdated Show resolved Hide resolved

bratpeki reviewed Dec 29, 2025

View reviewed changes

Wording change

0eb0f0a

Uh oh!

Conversation

steven-jaro commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steven-jaro commented May 22, 2025

Uh oh!

Uh oh!

steven-jaro commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

regulus79 commented May 22, 2025

Uh oh!

Uh oh!

steven-jaro commented May 22, 2025

Uh oh!

szeli1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steven-jaro commented May 23, 2025

Uh oh!

steven-jaro commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szeli1 left a comment

Choose a reason for hiding this comment

Uh oh!

szeli1 commented May 25, 2025

Uh oh!

steven-jaro commented May 25, 2025

Uh oh!

szeli1 left a comment

Choose a reason for hiding this comment

Uh oh!

JohannesLorenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

regulus79 commented Dec 9, 2025

Uh oh!

Rossmaxx commented Dec 27, 2025

Uh oh!

Uh oh!

JohannesLorenz commented Dec 27, 2025 • edited by bratpeki Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rossmaxx commented Dec 28, 2025

Uh oh!

bratpeki Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

bratpeki Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

bratpeki Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bratpeki Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

bratpeki commented Jan 25, 2026

Uh oh!

bratpeki commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

steven-jaro commented May 20, 2025 •

edited

Loading

steven-jaro commented May 22, 2025 •

edited

Loading

steven-jaro commented May 25, 2025 •

edited

Loading

JohannesLorenz commented Dec 27, 2025 •

edited by bratpeki

Loading