Skip to content
Open
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
2 changes: 2 additions & 0 deletions app/Community/Actions/FetchDynamicShortcodeContentAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public function execute(
array $gameIds = [],
array $hubIds = [],
array $eventIds = [],
string $convertedBody = '',
): ShortcodeDynamicEntitiesData {
return new ShortcodeDynamicEntitiesData(
users: $this->fetchUsers($usernames)->all(),
Expand All @@ -37,6 +38,7 @@ public function execute(
games: $this->fetchGames($gameIds)->all(),
hubs: $this->fetchHubs($hubIds)->all(),
events: $this->fetchEvents($eventIds)->all(),
convertedBody: $convertedBody,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ public function update(
// Convert [game={legacy_hub_id}] to [hub={game_set_id}].
$newPayload = Shortcode::convertLegacyGameHubShortcodesToHubShortcodes($newPayload);

// Convert [game=X?set=Y] to [game=backingGameId].
$newPayload = Shortcode::convertGameSetShortcodesToBackingGame($newPayload);

$comment->body = $newPayload;

// If this post is being edited by someone other than
Expand Down
26 changes: 20 additions & 6 deletions app/Community/Controllers/Api/ShortcodeApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use App\Community\Actions\FetchDynamicShortcodeContentAction;
use App\Community\Requests\PreviewShortcodeBodyRequest;
use App\Http\Controller;
use App\Support\Shortcode\Shortcode;
use Illuminate\Http\JsonResponse;

class ShortcodeApiController extends Controller
Expand All @@ -15,13 +16,26 @@ public function preview(
PreviewShortcodeBodyRequest $request,
FetchDynamicShortcodeContentAction $action,
): JsonResponse {
$body = $request->input('body');

// Normalize URLs to shortcode format (eg: "https://retroachievements.org/game/123" -> "[game=123]").
$body = normalize_shortcodes($body);

// Convert [game=X?set=Y] shortcodes to their backing game IDs.
$body = Shortcode::convertGameSetShortcodesToBackingGame($body);

// Extract entity IDs from the normalized+converted body.
$extractedIds = Shortcode::extractShortcodeIds($body);

// Fetch the entities and return the final converted body.
$entities = $action->execute(
usernames: $request->input('usernames'),
ticketIds: $request->input('ticketIds'),
achievementIds: $request->input('achievementIds'),
gameIds: $request->input('gameIds'),
hubIds: $request->input('hubIds'),
eventIds: $request->input('eventIds'),
convertedBody: $body,
achievementIds: $extractedIds['achievementIds'],
eventIds: $extractedIds['eventIds'],
gameIds: $extractedIds['gameIds'],
hubIds: $extractedIds['hubIds'],
ticketIds: $extractedIds['ticketIds'],
usernames: $extractedIds['usernames'],
);

return response()->json($entities);
Expand Down
1 change: 1 addition & 0 deletions app/Community/Data/ShortcodeDynamicEntitiesData.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public function __construct(
public array $games = [],
public array $hubs = [],
public array $events = [],
public string $convertedBody = '',
) {
}
}
11 changes: 1 addition & 10 deletions app/Community/Requests/PreviewShortcodeBodyRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,7 @@ class PreviewShortcodeBodyRequest extends FormRequest
public function rules(): array
{
return [
'usernames' => 'present|array',
'usernames.*' => 'string',
'ticketIds' => 'present|array',
'ticketIds.*' => 'integer',
'achievementIds' => 'present|array',
'achievementIds.*' => 'integer',
'gameIds' => 'present|array',
'gameIds.*' => 'integer',
'hubIds' => 'present|array',
'hubIds.*' => 'integer',
'body' => 'required|string',
];
}
}
6 changes: 6 additions & 0 deletions app/Helpers/database/forum.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ function editTopicComment(int $commentId, string $newPayload): void
// Convert [game={legacy_hub_id}] to [hub={game_set_id}].
$newPayload = Shortcode::convertLegacyGameHubShortcodesToHubShortcodes($newPayload);

// Convert [game=X?set=Y] to [game=backingGameId].
$newPayload = Shortcode::convertGameSetShortcodesToBackingGame($newPayload);

$comment = ForumTopicComment::findOrFail($commentId);
$comment->body = $newPayload;
$comment->save();
Expand Down Expand Up @@ -190,6 +193,9 @@ function submitTopicComment(
// Convert [game={legacy_hub_id}] to [hub={game_set_id}].
$commentPayload = Shortcode::convertLegacyGameHubShortcodesToHubShortcodes($commentPayload);

// Convert [game=X?set=Y] to [game=backingGameId].
$commentPayload = Shortcode::convertGameSetShortcodesToBackingGame($commentPayload);

// if this exact message was just posted by this user, assume it's an
// accidental double submission and ignore.
$latestPost = $user->forumPosts()->latest('created_at')->first();
Expand Down
21 changes: 17 additions & 4 deletions app/Helpers/shortcode.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,25 @@ function normalize_shortcodes(string $input): string
function normalize_targeted_shortcodes(string $input, string $kind, ?string $tagName = null): string
{
// Find any URL variants of user links and transform them into shortcode tags.
// First, handle URLs with a ?set= query param (these are games).
if ($kind === 'game') {
$findWithSet = [
"~https?://(?:[\w\-]+\.)?retroachievements\.org/game2?/([\w]{1,20})(?:-[^\s\"'<>]*)?(?:/)?\\?set=(\d+)~si",
"~https?://localhost(?::\d{1,5})?/game2?/([\w]{1,20})(?:-[^\s\"'<>]*)?(?:/)?\\?set=(\d+)~si",
];
$replaceWithSet = "[game=$1?set=$2]";
$input = preg_replace($findWithSet, $replaceWithSet, $input);
}

// Then, handle URLs without query params.
// Ignore URLs that contain path or query params.
// For games, match both /game/ and /game2/ URLs.
$pathPattern = $kind === 'game' ? 'game2?' : $kind;
$find = [
"~\<a [^/>]*retroachievements\.org/" . $kind . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))[^/>]*\][^</a>]*</a>~si",
"~\[url[^\]]*retroachievements\.org/" . $kind . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))[^\]]*\][^\[]*\[/url\]~si",
"~https?://(?:[\w\-]+\.)?retroachievements\.org/" . $kind . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))~si",
"~https?://localhost(?::\d{1,5})?/" . $kind . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))~si",
"~\<a [^/>]*retroachievements\.org/" . $pathPattern . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))[^/>]*\][^</a>]*</a>~si",
"~\[url[^\]]*retroachievements\.org/" . $pathPattern . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))[^\]]*\][^\[]*\[/url\]~si",
"~https?://(?:[\w\-]+\.)?retroachievements\.org/" . $pathPattern . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))~si",
"~https?://localhost(?::\d{1,5})?/" . $pathPattern . "/([\w]{1,20})(?:-[^\s\"'<>]*)?(/?(?![\w/?]))~si",
];
$replace = "[" . ($tagName ?? $kind) . "=$1]";

Expand Down
80 changes: 80 additions & 0 deletions app/Support/Shortcode/Shortcode.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use App\Models\System;
use App\Models\Ticket;
use App\Models\User;
use App\Platform\Actions\ResolveBackingGameForAchievementSetAction;
use App\Platform\Enums\GameSetType;
use Carbon\Carbon;
use Illuminate\Support\Facades\Cache;
Expand Down Expand Up @@ -128,6 +129,81 @@ public static function convertUserShortcodesToUseIds(string $input): string
}, $input);
}

public static function convertGameSetShortcodesToBackingGame(string $input): string
{
// Extract all [game=X?set=Y] or [game=X set=Y] patterns.
Copy link
Member

Choose a reason for hiding this comment

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

If the pattern is detected, the game portion is ignored.

[game=9999 set=9534]

still maps to the sonic subset.

Should the game association be validated?

Wouldn't it be easier to just have [set=9534]?

Copy link
Member Author

Choose a reason for hiding this comment

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

Logically, I think a [set=] shortcode is easier, but I'm afraid it may be more challenging from a UX POV:

  1. It's one more button.
  2. We don't expose the set IDs of base sets, which could make the functionality ambiguous or confusing.

Granted, [game=X?set=Y] doesn't feel good from a UX POV either.

Copy link
Member Author

Choose a reason for hiding this comment

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

The more I think on this, the more I'm wondering if it's worth simplifying this implementation significantly. Rather than messing with shortcodes, what if we just detect inline game set URLs (whether it's game or game2), and have this functionality only work on those things. That may be an easier UX to explain to folks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out it's really hard to rip out some kind of ?set=Y] shortcode handling and still have the Rube Goldberg machine work correctly.

In latest, I've resigned to us keeping the [game=X set=Y] handling, but we also support auto-processing of game2 URLs as suggested in other feedback. I'm hoping from a UX POV we can keep the "set=" shortcode stuff as an implementation detail, and not something we advertise to users.

Copy link
Member

Choose a reason for hiding this comment

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

If I include a [game=X set=Y] link in a post, all the other subset links point to set Y, regardless of what they're set to.

http://localhost:64000/game2/697?set=59

http://localhost:64000/game/697?set=59

http://localhost:64000/game/20026

[game=697 set=59]

[game=9999 set=59]

[game=697 set=9999]

The first five should all point at the same subset.

image

Oddly, removing the last line causes all the broken items to disappear.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Tricky, but should be addressed in latest.

preg_match_all('/\[game=(\d+)(?:\?|\s)set=(\d+)\]/i', $input, $matches, PREG_SET_ORDER);
if (empty($matches)) {
return $input;
}

// Collect all unique set IDs for bulk processing.
$setIds = array_unique(array_column($matches, 2));

// Now, resolve backing games for all sets.
// Build a map of achievement set ID -> backing game ID.
$setToBackingGameMap = [];
$resolveAction = (new ResolveBackingGameForAchievementSetAction());

foreach ($setIds as $setId) {
$backingGameId = $resolveAction->execute((int) $setId);
if ($backingGameId) {
$setToBackingGameMap[$setId] = $backingGameId;
}
}

// Replace each shortcode with the backing game ID.
// If no backing game is found, fall back to the original game ID.
return preg_replace_callback(
'/\[game=(\d+)(?:\?|\s)set=(\d+)\]/i',
function ($match) use ($setToBackingGameMap) {
$originalGameId = $match[1];
$setId = $match[2];

$gameId = $setToBackingGameMap[$setId] ?? $originalGameId;

return "[game={$gameId}]";
},
$input
);
}

public static function extractShortcodeIds(string $input): array
{
// Extract achievement IDs from [ach=X] shortcodes.
preg_match_all('/\[ach=(\d+)\]/i', $input, $achievementMatches);
$achievementIds = array_unique(array_map('intval', $achievementMatches[1]));

// Extract game IDs from [game=X] shortcodes (but not [game=X set=Y]).
preg_match_all('/\[game=(\d+)(?!\?|\ set=)\]/i', $input, $gameMatches);
$gameIds = array_unique(array_map('intval', $gameMatches[1]));

// Extract hub IDs from [hub=X] shortcodes.
preg_match_all('/\[hub=(\d+)\]/i', $input, $hubMatches);
$hubIds = array_unique(array_map('intval', $hubMatches[1]));

// Extract event IDs from [event=X] shortcodes.
preg_match_all('/\[event=(\d+)\]/i', $input, $eventMatches);
$eventIds = array_unique(array_map('intval', $eventMatches[1]));

// Extract ticket IDs from [ticket=X] shortcodes.
preg_match_all('/\[ticket=(\d+)\]/i', $input, $ticketMatches);
$ticketIds = array_unique(array_map('intval', $ticketMatches[1]));

// Extract usernames from [user=X] shortcodes.
preg_match_all('/\[user=([^\]]+)\]/i', $input, $userMatches);
$usernames = array_unique($userMatches[1]);

return [
'achievementIds' => array_values($achievementIds),
'gameIds' => array_values($gameIds),
'hubIds' => array_values($hubIds),
'eventIds' => array_values($eventIds),
'ticketIds' => array_values($ticketIds),
'usernames' => $usernames,
];
}

public static function stripAndClamp(
string $input,
int $previewLength = 100,
Expand Down Expand Up @@ -266,6 +342,10 @@ private function prefetchUsers(string $input): void

private function parse(string $input, array $options = []): string
{
// Resolve game set shortcodes to backing games before render-time.
// This allows [game=1?set=9534] to display the correct backing game.
$input = self::convertGameSetShortcodesToBackingGame($input);

$this->prefetchUsers($input);

// make sure to use attribute delimiter for string values
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,13 @@ import { useMutation } from '@tanstack/react-query';
import axios from 'axios';
import { route } from 'ziggy-js';

import type {
DynamicShortcodeEntities,
ShortcodeBodyPreviewMutationResponse,
} from '@/common/models';
import type { ShortcodeBodyPreviewMutationResponse } from '@/common/models';

export function useShortcodeBodyPreviewMutation() {
return useMutation({
mutationFn: (payload: DynamicShortcodeEntities) =>
axios.post<ShortcodeBodyPreviewMutationResponse>(
route('api.shortcode-body.preview'),
payload,
),
mutationFn: (body: string) =>
axios.post<ShortcodeBodyPreviewMutationResponse>(route('api.shortcode-body.preview'), {
body,
}),
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('Hook: useHydrateShortcodeDynamicEntities', () => {
events: [],
tickets: [],
users: [],
convertedBody: '',
};

// ACT
Expand All @@ -45,6 +46,7 @@ describe('Hook: useHydrateShortcodeDynamicEntities', () => {
events: [],
tickets: [],
users: [],
convertedBody: '',
},
},
},
Expand All @@ -65,6 +67,7 @@ describe('Hook: useHydrateShortcodeDynamicEntities', () => {
hubs: [{ id: 3 }],
tickets: [{ id: 4 }],
users: [{ id: 5 }],
convertedBody: '',
};

// ACT
Expand All @@ -77,6 +80,7 @@ describe('Hook: useHydrateShortcodeDynamicEntities', () => {
events: [],
tickets: [],
users: [],
convertedBody: '',
},
},
});
Expand Down
Loading