thumbnail fix - will this WORK THIS TIME? #154

Merged
ic3w0lf22 merged 4 commits from thumbnail-fix-1 into staging 2025-06-06 02:51:36 +00:00
Member

pls no conflicts thanks

pls no conflicts thanks
ic3w0lf22 added 1 commit 2025-06-04 02:03:43 +00:00
thumbnail fix - will this WORK THIS TIME?
All checks were successful
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is passing
762ee874a0
Owner

You can update the same pull request next time, no need for a new one

You can update the same pull request next time, no need for a new one
Author
Member

Noted. (I have no idea how to use git other than pull & push)

Noted. (I have no idea how to use git other than pull & push)
Quaternions requested changes 2025-06-04 02:16:04 +00:00
Dismissed
@@ -0,0 +1,28 @@
import path from 'path'
import { promises as fs } from 'fs'
export async function errorImageResponse(statusCode: number = 500): Promise<Response> {
Owner

Why does errorImageResponse return Promise when it is already marked as async? (I don't know if this is normal in javascript)

Why does errorImageResponse return Promise when it is already marked as `async`? (I don't know if this is normal in javascript)
Author
Member

ngl chatgpt wrote this entire file

ngl chatgpt wrote this entire file
Owner

Ok this is just how it's done in javascript

Ok this is just how it's done in javascript
Quaternions marked this conversation as resolved
@@ -0,0 +6,4 @@
const filePath = path.join(process.cwd(), 'public/errors', file)
try {
const buffer = await fs.readFile(filePath)
Owner

Why are you reading a file into a buffer instead of using html?

Why are you reading a file into a buffer instead of using html?
Author
Member

I will get co-pilot to add this

I will get co-pilot to add this
Author
Member

My previous comment was a lie (maybe), in this case, the errors are specifically for thumbnails, and whatever files I had that used errorImageResponse don't exist since I messed up my local repo git stuff

My previous comment was a lie (maybe), in this case, the errors are specifically for thumbnails, and whatever files I had that used `errorImageResponse` don't exist since I messed up my local repo git stuff
Owner

Oh I see, it's returning the 404 image as a map thumbnail. So the logic here is populating the cache with the 404 image for that map.

Oh I see, it's returning the 404 image as a map thumbnail. So the logic here is populating the cache with the 404 image for that map.
Quaternions marked this conversation as resolved
ic3w0lf22 added 1 commit 2025-06-04 02:42:47 +00:00
Implement errorImageResponse
Some checks failed
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is failing
e1fc637619
ic3w0lf22 added 1 commit 2025-06-04 02:53:09 +00:00
Fix error & include error message in response headers
All checks were successful
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is passing
8d5bd9e523
Owner

Press "Resolve conversation" when you fix each one

Press "Resolve conversation" when you fix each one
Owner

Where is the image cache invalidated? Is the memory ever freed?

Where is the image cache invalidated? Is the memory ever freed?
Author
Member

It never is, but shouldn’t be an issue yet since there’s only like 200 maps that would ever be in that cache. I’ll implement something to free the memory soon.

It never is, but shouldn’t be an issue yet since there’s only like 200 maps that would ever be in that cache. I’ll implement something to free the memory soon.
Owner

The maps on https://maps.staging.strafes.net/maps only show a few thumbnails for me, it adds a new thumbnail sometimes when I reload the page (not using the changes in this PR)
image.png

The maps on https://maps.staging.strafes.net/maps only show a few thumbnails for me, it adds a new thumbnail sometimes when I reload the page (not using the changes in this PR) ![image.png](/attachments/e46da241-7baa-47c7-a97e-f79862b629c4)
244 KiB
Quaternions approved these changes 2025-06-04 05:30:44 +00:00
Dismissed
Owner

Merge as-is, or implement cache eviction and I can re-review

Merge as-is, or implement cache eviction and I can re-review
ic3w0lf22 added 1 commit 2025-06-05 23:42:46 +00:00
implement cache de-exister
All checks were successful
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is passing
3f848a35c8
ic3w0lf22 dismissed Quaternions's review 2025-06-05 23:42:47 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Author
Member

Once these changes are pushed to staging, it should fix the thumbnails issue. Hopefully.
-pray emoji

Once these changes are pushed to staging, it _should_ fix the thumbnails issue. Hopefully. -pray emoji
ic3w0lf22 scheduled this pull request to auto merge when all checks succeed 2025-06-05 23:47:09 +00:00
dowoge approved these changes 2025-06-06 00:12:20 +00:00
Dismissed
dowoge left a comment
Member

what happens if I approve

Edit: absolutely nothing it turns out
im a noob

what happens if I approve Edit: absolutely nothing it turns out im a noob
itzaname requested review from Quaternions 2025-06-06 01:22:19 +00:00
itzaname dismissed dowoge's review 2025-06-06 01:22:28 +00:00
Reason:

GTFO

Member

u suck wheres the mod website

u suck wheres the mod website
Quaternions approved these changes 2025-06-06 02:51:35 +00:00
ic3w0lf22 merged commit 07391a84cb into staging 2025-06-06 02:51:36 +00:00
Sign in to join this conversation.