perf: denormalize games.parent_game_id#4891
Conversation
| } | ||
|
|
||
| return $this->parentGameId !== null; | ||
| return $this->parent_game_id !== null; |
There was a problem hiding this comment.
The short-circuit can be removed now.
| public function parent(): BelongsTo | ||
| { | ||
| return $this->belongsTo(Game::class, 'parent_game_id'); | ||
| } |
There was a problem hiding this comment.
Is there a reason to add this instead of updating parentGame? As best as I can tell, there's only two references to parentGame(), and those can be updated by just removing the parentheses.
There was a problem hiding this comment.
Updating parentGame is better. Done in latest.
There was a problem hiding this comment.
I'm not sure if this isn't working, or if I'm missing something.
- Open game 4: http://localhost:64000/manage/games/4?relation=1
- Associate existing subset 25811
- View game page: http://localhost:64000/game/4 (subset visible)
- Query database:
MariaDB [retroachievements-web]> select parent_game_id from games where id=25811;
+----------------+
| parent_game_id |
+----------------+
| 25807 |
+----------------+
1 row in set (0.001 sec)
I think it should return 4.
There was a problem hiding this comment.
This is confusing, and your expectation is reasonable, but I think what's happening right now that you're seeing is the correct behavior.
It's a known limitation that the field is lossy. In other words, only one game ID is supported in this field, so subsets like Pokemon R/B's won't necessarily be accurate. I think the trade-off is fine for the time being.
In this case, game 25807 (the real parent game where this subset was first attached) is the earliest non-core link as the parent. Therefore, it "wins" over the new one to game 4. This preserves existing prod behavior.
| $retroRatioPlayerCount = Game::find($game->parentGameId)->players_hardcore ?? 0; | ||
| if ($game->parent_game_id) { | ||
| $retroRatioPlayerCount = Game::find($game->parent_game_id)->players_hardcore ?? 0; | ||
| } |
There was a problem hiding this comment.
Might be better to use relationship in case it's already loaded?
$game->loadMissing('parentGame');
if ($game->parentGame) {
$retroRatioPlayerCount = $game->parentGame->players_hardcore ?? 0;
}
2 participants
Follow-up to hotfix from #4887.
This PR adds a
games.parent_game_iddenormalized field and largely deletes the hotfix code. The field is written in-place during the migration itself. Observers keep the field synced.It's a known limitation that the field is lossy. In other words, only one game ID is supported in this field, so subsets like Pokemon R/B's won't necessarily be accurate. I think the trade-off is fine for the time being.