Skip to content

Conversation

WesleyMPG
Copy link
Contributor

@WesleyMPG WesleyMPG commented May 5, 2021

So, class naming first? What about PlayGui?

@RMPR
Copy link
Owner

RMPR commented May 7, 2021 via email

Previously playCtrl had part of gui handling, and functionality due to
untoggle the play button when playback stops.

Make play_once and play_many be just one function

Before when many playbacks were needed, play_many was executed in
its own thread and created a thread for play_once. But now just a
"play_once" thread is created and it calls a playback how many
times are needed
self._stop_locks[0] = True


class PlayInterface:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name might be confusing for a newcomer in the code base, since in OOP an interface is something with a well-defined meaning and more importantly that cannot be instantiated.

atbswp/gui.py Outdated

# play_button_ctrl
self.pbc = control.PlayCtrl()
self.pbc = control.PlayInterface()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View previous comment

@RMPR
Copy link
Owner

RMPR commented May 8, 2021

So, class naming first? What about PlayGui?

You can also just swap the names, I think it still makes sense.

@RMPR
Copy link
Owner

RMPR commented May 8, 2021

Just found a bug in infinite playback with these changes. Steps to
reproduce:

  • Record a capture
  • Enable infinite playback
  • Replay the capture
  • Click on the play button to stop (or press F10 the default shortcut)

Expected behaviour: The capture stops instantly or at least stops after
the current iteration.

Observed behaviour: The capture keeps running forced to kill the
process.

@WesleyMPG
Copy link
Contributor Author

Observed behaviour: The capture keeps running forced to kill the
process.

That's weird. Here it's working fine:

@RMPR
Copy link
Owner

RMPR commented May 8, 2021 via email

@RMPR
Copy link
Owner

RMPR commented May 8, 2021

In the meantime I implemented the event-based solution we discussed here
gui-control

Due to the refactoring of PlayCtrl, the playback hotkey was not
working because its event was still being handled according to the
previous way.
@WesleyMPG
Copy link
Contributor Author

In the meantime I implemented the event-based solution we discussed here

I'll check that.

@RMPR
Copy link
Owner

RMPR commented May 9, 2021

A rebase on top of develop would be great, I modified a bit the PlayCtrl, so you might have to solve merge issues.

@RMPR RMPR force-pushed the develop branch 2 times, most recently from ca8f39d to 49a7f0a Compare January 10, 2023 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants