Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions Explorer/Assets/DCL/Backpack/AvatarSection/AvatarController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ public class AvatarController : ISection, IDisposable
private readonly BackpackGridController backpackGridController;
private readonly AvatarTabsManager tabsManager;
private readonly URLBuilder urlBuilder = new ();
private readonly URLParameter marketplaceSourceParam = new ("utm_source", "backpack");
// Still "backpack" even though the destination is now the Shop: this names where the click came FROM,
// and keeping it is what lets the web side see this traffic move from the marketplace over to the Shop.
private readonly URLParameter shopSourceParam = new ("utm_source", "backpack");
Comment on lines +28 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Comment narrates external behavior (CLAUDE.md §11: "A comment must state only what the annotated code itself does or guarantees, never what callers or upper layers will do"). The second line describes what "the web side" does with the UTM parameter — that behavior is external and could change independently, making the comment stale.

Suggested change
// Still "backpack" even though the destination is now the Shop: this names where the click came FROM,
// and keeping it is what lets the web side see this traffic move from the marketplace over to the Shop.
private readonly URLParameter shopSourceParam = new ("utm_source", "backpack");
// Identifies the originating surface, not the destination — value stays "backpack" across redirects.
private readonly URLParameter shopSourceParam = new ("utm_source", "backpack");


public AvatarController(AvatarView view,
UnityAppWebBrowser webBrowser,
Expand All @@ -51,7 +53,7 @@ public AvatarController(AvatarView view,

rectTransform = view.GetComponent<RectTransform>();

view.marketplaceButton.onClick.AddListener(OnOpenMarketplace);
view.marketplaceButton.onClick.AddListener(OnOpenShop);

slotsController = new BackpackSlotsController(slotViews,
backpackCommandBus,
Expand All @@ -72,11 +74,14 @@ public AvatarController(AvatarView view,
tabsManager.InitializeAndEnable();
}

private void OnOpenMarketplace()
// The Shop and not the marketplace: this button lives in the avatar section, so what it is asked for is
// always wearables or emotes, and those are what the Shop sells. LAND keeps pointing at the marketplace
// elsewhere — the Shop carries no parcels or estates.
Comment on lines +77 to +79

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] The second and third lines narrate behavior of other code paths ("LAND keeps pointing at the marketplace elsewhere") — this is external behavior per CLAUDE.md §11. The first clause, that the button's context is always wearables/emotes, is a valid guarantee of the annotated method.

Suggested change
// The Shop and not the marketplace: this button lives in the avatar section, so what it is asked for is
// always wearables or emotes, and those are what the Shop sells. LAND keeps pointing at the marketplace
// elsewhere — the Shop carries no parcels or estates.
// Opens the Shop: this button lives in the avatar section, so the context is always wearables or emotes.

private void OnOpenShop()
Comment on lines +77 to +80

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Over-explanatory comment (CLAUDE.md §11 — comments must explain what the annotated code does, not narrate caller/external behavior). OnOpenShop + DecentralandUrl.ShopLink already communicate the destination. The comment narrates the avatar section's UI content, what the Shop sells, and what happens with LAND — all external to this method.

Suggested change
// The Shop and not the marketplace: this button lives in the avatar section, so what it is asked for is
// always wearables or emotes, and those are what the Shop sells. LAND keeps pointing at the marketplace
// elsewhere — the Shop carries no parcels or estates.
private void OnOpenShop()
private void OnOpenShop()

{
urlBuilder.Clear();
urlBuilder.AppendDomain(URLDomain.FromString(decentralandUrlsSource.Url(DecentralandUrl.Market)));
urlBuilder.AppendParameter(marketplaceSourceParam);
urlBuilder.AppendDomain(URLDomain.FromString(decentralandUrlsSource.Url(DecentralandUrl.ShopLink)));
urlBuilder.AppendParameter(shopSourceParam);
webBrowser.OpenUrlMainThreadOnly(urlBuilder.Build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private void BuyWearableButtonClicked()
async UniTaskVoid AnimateAndAwaitAsync()
{
await UniTask.Delay((int)(view.buyButtonAnimationDuration * 1000));
webBrowser.OpenUrlMainThreadOnly(currentWearable.GetMarketplaceLink(decentralandUrlsSource));
webBrowser.OpenUrlMainThreadOnly(currentWearable.GetShopLink(decentralandUrlsSource));
MarketClicked?.Invoke();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,20 @@ namespace DCL.InWorldCamera.PhotoDetail
public static class PhotoDetailUtility
{
/// <summary>
/// Extracts the marketplace link from a wearable.
/// Taken from the old renderer.
/// Builds the Shop link for a wearable worn in a photo.
/// <para>
/// The Shop and not the marketplace: the subject here is always a wearable, which is what the Shop
/// sells and prices in USD. LAND still belongs to the marketplace — the Shop carries no parcels.
/// </para>
/// <para>
/// Note the route differs, so this is not a host swap: the marketplace addresses an item as
Comment on lines +13 to +17

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] The first <para> narrates the Shop's product scope ("LAND still belongs to the marketplace") — external behavior per CLAUDE.md §11 that could change independently of this method. The summary line already establishes this builds a Shop link for a wearable. Remove this paragraph; the second <para> (route difference) documents what the method itself does and is the valuable part.

Suggested change
/// The Shop and not the marketplace: the subject here is always a wearable, which is what the Shop
/// sells and prices in USD. LAND still belongs to the marketplace — the Shop carries no parcels.
/// </para>
/// <para>
/// Note the route differs, so this is not a host swap: the marketplace addresses an item as
/// Builds the Shop link for a wearable worn in a photo.

/// <c>/contracts/{contract}/items/{id}</c> and the Shop as <c>/item/{contract}/{id}</c>. Keeping the
/// old shape against the new host is a 404, not a redirect.
/// </para>
/// </summary>
public static string GetMarketplaceLink(this IWearable wearable, IDecentralandUrlsSource decentralandUrlsSource)
public static string GetShopLink(this IWearable wearable, IDecentralandUrlsSource decentralandUrlsSource)
Comment on lines +13 to +22

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Verbose XML doc (CLAUDE.md §11). The first <para> narrates business context about what the Shop sells — external to this method. The route-shape note is genuinely useful (it prevents reverting to the marketplace path pattern), but can be condensed into the summary line.

Suggested change
/// The Shop and not the marketplace: the subject here is always a wearable, which is what the Shop
/// sells and prices in USD. LAND still belongs to the marketplace — the Shop carries no parcels.
/// </para>
/// <para>
/// Note the route differs, so this is not a host swap: the marketplace addresses an item as
/// <c>/contracts/{contract}/items/{id}</c> and the Shop as <c>/item/{contract}/{id}</c>. Keeping the
/// old shape against the new host is a 404, not a redirect.
/// </para>
/// </summary>
public static string GetMarketplaceLink(this IWearable wearable, IDecentralandUrlsSource decentralandUrlsSource)
public static string GetShopLink(this IWearable wearable, IDecentralandUrlsSource decentralandUrlsSource)
/// Builds the Shop link for a wearable worn in a photo. The route is
/// <c>/item/{contract}/{id}</c>, not the marketplace's <c>/contracts/{contract}/items/{id}</c>.

{
var marketplace = $"{decentralandUrlsSource.Url(DecentralandUrl.Market)}/contracts/{{0}}/items/{{1}}";
var shop = $"{decentralandUrlsSource.Url(DecentralandUrl.ShopLink)}/item/{{0}}/{{1}}";
ReadOnlySpan<char> idSpan = wearable.GetUrn().ToString().AsSpan();
int lastColonIndex = idSpan.LastIndexOf(':');

Expand All @@ -29,7 +37,7 @@ public static string GetMarketplaceLink(this IWearable wearable, IDecentralandUr
if (!contract.StartsWith("0x") || !int.TryParse(item, out int _))
return "";

return string.Format(marketplace, contract, item);
return string.Format(shop, contract, item);
}
}
}
Loading