Skip to content

play a file from the shared file tab - #278

Closed
tbo47 wants to merge 1 commit into
amule-org:masterfrom
tbo47:play
Closed

play a file from the shared file tab#278
tbo47 wants to merge 1 commit into
amule-org:masterfrom
tbo47:play

Conversation

@tbo47

@tbo47 tbo47 commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

Play a file from the shared file tab.

I tried to use the feedback from
#272

@got3nks

got3nks commented Jul 3, 2026

Copy link
Copy Markdown

Thanks @tbo47 — this addresses most of the #272 points: the preview logic is now a single shared PreviewFile instead of a third copy, Delete is split out, the macOS VLC prefill is gone, and the title says what it does. Good iteration.

A few things before it can go in:

1. Gate Play/Open to the monolithic app — on both tabs. The new Shared-tab item (EVT_MENU(MP_VIEW…), the menu entry, and OnPlayFile) isn't wrapped in #ifndef CLIENT_GUI. In amulegui the paths are the daemon's, so it opens a nonexistent local path. Please add the gate — and since you're in this code, the existing Download-tab preview (OnPreviewFile / its MP_VIEW) has the same gap, so give it the same #ifndef CLIENT_GUI gate while we're here.

2. Guard the downcast. static_cast<CPartFile *>(file) (in the menu enable-check and in PreviewFile's incomplete branch) is only valid because an incomplete shared file is always a CPartFile. That invariant holds today, but the raw downcast is a latent crash if it ever changes — please add an IsPartFile() guard so it's explicit and safe.

3. Replace the reinterpret_cast. reinterpret_cast<CKnownFile *>(GetItemData(index)) in OnPlayFile is heavier than needed and unsafe if the stored type ever differs — please use a proper static_cast matching how the item pointer is stored/retrieved elsewhere in this control.

4. Label it "Open" on the Shared tab and route by file type — aMule already has the helper. The Shared list is arbitrary files, not just media, so a completed file should say "Open". Right now the configured video player is forced on every type, so "Play" on a non-media file (a .zip, a document) launches the video player on it. Classify with GetFiletype(file->GetFileName()) from OtherFunctions.h — the same helper CPartFile::PreviewAvailable() already uses — and route: ftVideo / ftAudio → the configured video player (or the OS opener if none is set); everything else → the OS default opener (open / xdg-open / start) regardless of the video-player setting. (Incomplete shared partfiles are media-in-progress, so previewing those via the player as today is fine.)

5. The po diff. The large catalog churn is because your branch predates recent string additions on master — the #: source line numbers have all shifted. Please rebase onto current master and regenerate the catalogs as the final step, so the diff shows just your new string, in sync with the tree.

LSalami added a commit to LSalami/amule that referenced this pull request Jul 27, 2026
got3nks's review of amule-org#650 asked for this to reach amulegui and amuleapi,
not just the monolithic app, ahead of the push to keep every feature
available across all three clients.

Adds two optional EC_TAG_CONNSTATE sub-tags, hand-synced into
ECCodes.abstract/.h per the project's existing convention (never
regenerate the committed header -- it reflows the whole license block):

- EC_TAG_ED2K_CONNECTED_SINCE / EC_TAG_KAD_CONNECTED_SINCE (uint32 unix
  timestamps), populated in CEC_ConnState_Tag's constructor from
  CamuleApp's existing m_ed2kConnectedSince/m_kadConnectedSince, present
  only while actually connected.

Consumption side:
- CamuleRemoteGuiApp (amulegui) gets matching m_ed2kConnectedSince/
  m_kadConnectedSince members and GetED2KConnectedSince()/
  GetKadConnectedSince() accessors -- same names as CamuleApp's, so
  ServerWnd.cpp's "Connected since" rows no longer need a
  #ifndef CLIENT_GUI gate; each build's theApp resolves to whichever
  class actually has the data.
- amuleapi's Refresher.cpp reads the same sub-tags directly into
  StatusSnapshot.{ed2k,kad}_connected_since, surfaced in GET /status as
  ed2k.connected_since / kad.connected_since (0 while disconnected).
  Documented in docs/api/REFERENCE.md.

po/ catalogs regenerated as the final step (scripts/update-po.sh) so
the diff is just the "Connected since:" string in sync with the tree,
per got3nks's note on amule-org#278 about avoiding line-number-shift churn.

Verified by building and running all three targets (amule, amulegui,
amuleapi) plus the full unit test suite (26/26 passing, including
RefresherTest which exercises the new StatusSnapshot fields directly).
@tbo47 tbo47 closed this Aug 6, 2026
got3nks added a commit that referenced this pull request Aug 7, 2026
…ts (#831)

Supersedes #278 by tbo47, whose Shared-tab "play" item this grew out of; its
review points (guard the CPartFile downcast, drop the reinterpret_cast, label
it Open and route by file type) are carried over.

Adds Open and "Show in file manager" to the Downloads and Shared Files lists.
Availability is now whether the resolved path exists on this host, tested when
the menu is built, rather than which binary is running -- the old check was
build variant plus loopback locality, which refused every shared-filesystem
setup and allowed a local file that had since been deleted. One filesystem
check answers both entries.

Media goes to the configured video player, including the in-progress preview;
everything else goes to the platform opener regardless of that setting, so
opening an archive no longer launches the video player. An unfinished download
is NNNN.part on disk and no desktop handles that extension, so with no player
configured that case says so instead of failing twice. Revealing is suppressed
for unfinished downloads.

The player template is split into arguments once -- UNIX rules on Unix, DOS on
Windows -- and the placeholders substituted inside the resulting arguments in a
single pass, so the path crosses as one argv entry. Building a command string
instead would let a remote-supplied eD2k filename inject arguments into the
player, and escaping quotes (what the old code did) is bypassable.

macOS reveals via NSWorkspace, Windows via explorer /select,, Linux opens the
containing folder. Launches keep the AppImage-safe environment from #334 and
are guarded against an escaping exception, which would otherwise terminate
aMule with no signal, no backtrace and no crash report.

Both lists pin the context menu to the item rather than the row, so a list
mutated while the menu is open cannot redirect the click to a neighbour.

Also fixes a Downloads label that fell through to an empty string, and an
enable test that used the media-only PreviewAvailable() -- so a finished .zip
or .pdf could not be opened at all.
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