Opened 12 years ago

Closed 9 years ago

Last modified 9 years ago

#2794 closed Enhancement (fixed)

Don't apply auto-grouping until torrent is demagnetized

Reported by: skurtix Owned by: evands
Priority: Normal Milestone: 2.74
Component: Mac Client Version: 1.82
Severity: Minor Keywords: magnet group condition
Cc: elpasi

Description

If you add a magnet-link torrent it will bypass the auto-group conditions and be added without a group. (latest snow leopard, transmission 1.82, added from latest Safari)

Attachments (1)

fix_magnet_link_grouping.patch (16.7 KB) - added by evands 9 years ago.
Patch to fix reanalyzing groups/destinations for magnet links

Download all attachments as: .zip

Change History (35)

comment:1 Changed 12 years ago by skurtix

  • Milestone changed from None Set to 1.83

comment:2 Changed 12 years ago by livings124

  • Milestone changed from 1.83 to None Set
  • Priority changed from High to Normal
  • Resolution set to worksforme
  • Severity changed from Major to Normal
  • Status changed from new to closed

When you set the milestone, you're either give a patch or giving us marching orders. And I just tested - my autogroup rule for name was applied just fine.

comment:3 Changed 12 years ago by Crypticus

  • Resolution worksforme deleted
  • Status changed from closed to reopened

When a magnet link is added, while it is in it's red state (Locating the file info via the hash??) spaces and dots appear to be replaced by '+' characters. It appears that the code the performs the conditional matching is done before the torrent is downloaded from the DHT and the proper name of the file is generated. This causes the conditional group matching to fail on anything other than the most simplistic of matching.

Example: Condition is set to match "Name contains" = fringe.

This works for both the magnet link and downloading the actual torrent file.

However, Condition is set to match "Name contains" = how.i.met.your.mother

This work for the torrent, fails for the magnet link.

Proposed solution is to change to order in which conditions are tested again. Wait till after the torrent leave the red state, then test against conditionals.

As a side note it would be nice if there was a menu option to retest all torrent in list against the conditions, in the event you add a bunch of torrents, then decide to make a new group, this would allow transmisson to regroup automatically.

comment:4 Changed 12 years ago by livings124

  • Severity changed from Normal to Minor
  • Summary changed from Group conditions doesn't affect magnet-added torrents to Don't apply auto-grouping until torrent is demagnetized

comment:5 Changed 12 years ago by livings124

  • Component changed from Transmission to Mac Client

comment:6 Changed 12 years ago by skurtix

Sorry about the mistakes in the ticket livings, but there is indeed a problem with auto-group conditions and magnet links.

comment:7 Changed 12 years ago by elpasi

I made a ticket about this, but it was closed for being a dupe. I've found that:

Not only is it noticeable if matching by name and the dots become plusses (which I have not encountered), but also if the magnet link that is distributed doesn't contain a &dn= parameter at all, in which case all my name based rules fail miserably.

comment:8 Changed 12 years ago by elpasi

  • Cc elpasi added

comment:9 Changed 11 years ago by yusf

+1 For fixing this!

comment:10 Changed 10 years ago by livings124

#4845 is a duplicate, but provides a nice potential change.

comment:11 Changed 10 years ago by evands

Change-in-progress from #4845 (not claimed to be a complete or workable solution yet):

Index: macosx/Torrent.m
===================================================================
--- macosx/Torrent.m	(revision 13257)
+++ macosx/Torrent.m	(working copy)
@@ -1869,6 +1869,16 @@
     
     [self createFileList];
     
+	if ([self groupValue] == -1)
+		[self setGroupValue:[[GroupsController groups] groupIndexForTorrent: self]];
+
+	//change the location if the group calls for it and it's not already set
+	if (!tr_torrentGetCurrentDir(fHandle) && [[GroupsController groups] usesCustomDownloadLocationForIndex: [self groupValue]])
+	{
+		NSString *location = [[GroupsController groups] customDownloadLocationForIndex: [self groupValue]];
+		[self changeDownloadFolderBeforeUsing: location];
+	}
+	
     [[NSNotificationCenter defaultCenter] postNotificationName: @"ResetInspector" object: self];
 }

livings124 wrote:

...The patch looks pretty good, though. My main concern is that manually-set groups/locations will be lost when it's demagnetized. How do we strike a balance between re-applying groups and respecting user settings?

I included the if ([self groupValue] == -1) for that purpose; if a group has been assigned somehow, that'll no longer be the default -1 and we won't change anything.

Similarly, I checked the current directory for the torrent to avoid overwriting a manually selected download location. I'm not sure if architecturally that'll work as expected for a magnet link, though -- at what point is the planned download location for a magnet link resolved? I couldn't determine it in a brief perusal of the source.

comment:12 follow-up: Changed 10 years ago by livings124

I'm not sure that the groupValue check would cut it. What if it was put automatically in the wrong group before, for example? Maybe that shouldn't be a concern, though.

The location set for the magnet link wouldn't be used before demagnetizing, so that shouldn't be a concern. I'm not sure when the value is set in code, either, though.

comment:13 in reply to: ↑ 12 Changed 10 years ago by evands

Replying to livings124:

I'm not sure that the groupValue check would cut it. What if it was put automatically in the wrong group before, for example? Maybe that shouldn't be a concern, though.

Good point. Perhaps we need to create a separate variable to track if this groupValue was user assigned or automatically determined, and if it was automatic (or not determined at all) then run the group assignment process.

comment:14 Changed 10 years ago by livings124

That's what I was thinking, too, at least in terms of which group its in. Still not sure in regards to location. Perhaps in the magnet add window it should be an option?

comment:15 Changed 10 years ago by evands

Hate to increase complexity with an extra option... how about if we check the location versus the default (the user's downloads directory, I assume? Haven't checked), and if it's still the default, we assume it's okay to follow group-location assignments, and if it's not, then it probably isn't because the user explicitly picked a new location.

comment:16 Changed 10 years ago by livings124

Perhaps changing the location could set off the new flag, same as changing the group value.

comment:17 Changed 10 years ago by evands

Yeah, I like that.

If that plan sounds good, I can put together a patch to implement this. It'll be a couple weeks before I have time to dive into it; no offense taken if you're interested and want to do it first.

comment:18 Changed 10 years ago by evands

  • Owner set to evands
  • Status changed from reopened to new

comment:19 Changed 10 years ago by livings124

Sounds good evands!

comment:20 Changed 10 years ago by jordan

evands: any news?

comment:21 Changed 10 years ago by evands

I haven't had a chance to work on this yet as I've mostly been on the road, but I still plan to.

comment:22 Changed 10 years ago by jordan

Okay. Thanks for checking in :)

comment:23 Changed 9 years ago by jurek

Any news? I just checked and in 2.51 on mac the bug still persists to annoy.. :-) As other people, I have a customized group based on "any file contains avi/mkv/mp4" and it works perfectly with .torrents, but won't with magnets. Please fix it:)

comment:24 Changed 9 years ago by TheDov

I'm noticing this bug, too. Any updates? I'm on 2.61

comment:25 Changed 9 years ago by livings124

evands: any news with this?

comment:26 Changed 9 years ago by evands

Really, it's been 5 months? Time flies. Never again can I complain about someone following up really-late after promising to work on something...

I'm alive and still interested in fixing this bug. I don't have any progress to report because I've gotten caught up in a million other things. Thanks for the ping, and if nobody beats me to it I'll hopefully have a chance to knock it out soon.

comment:27 Changed 9 years ago by jurek

Thank you!

comment:28 Changed 9 years ago by livings124

Sounds good!

Changed 9 years ago by evands

Patch to fix reanalyzing groups/destinations for magnet links

comment:29 Changed 9 years ago by evands

fix_magnet_link_grouping.patch fixes this issue and the potential concerns brought up by livings124 above, by tracking whether group and download folder changes are initiated by the user or automatically determined. When a magnet link's tracker data is downloaded, each of group and download location are redetermined, each only if it was not manually set by the user. The UI (in the Mac app) is properly updated when the group changes.

Patch is against [13561].

comment:30 Changed 9 years ago by livings124

  • Milestone changed from None Set to 2.80

Thanks! I'll take a closer look and should get it in for 2.8.

comment:31 Changed 9 years ago by livings124

The one thing I noticed is that in the add window, when setting the initial location (AddWindowController:130) you set the location with TorrentDeterminationAutomatic?. Should it be TorrentDeterminationUserSpecified? if fLockDestination is YES?

comment:32 Changed 9 years ago by livings124

  • Resolution set to fixed
  • Status changed from new to closed
  • Type changed from Bug to Enhancement

I've made the change in the above comment and submitted in r13602. If that's not right let me know. Thanks for the patch!

comment:33 Changed 9 years ago by evands

Tracing the path of invocation, fLockDestination is only YES if the @"OpenCreatedTorrentFile" notification was posted and specified a path. That only happens from the CreatorWindowController. I think that means that your change to my patch is correct.

comment:34 Changed 9 years ago by jordan

  • Milestone changed from 2.80 to 2.74
Note: See TracTickets for help on using tickets.