[Feat] Add folder uploading #33

jiyuu-jin · 2024-09-09T19:26:00Z

Description

Adds folder uploading via the new HP uploads API.
Adds the webgl platform with a path which (webgl does not require the executable, zip or any other fields).

Testing

You can test by adding a webgl field with a folder path to a hyperplay.yml platforms:

account: test-ground
project: test4
release: 0.0.5

platforms:
  darwin_amd64:
    path: dist/darwin/amd64/Kosium0.1.7Win.zip
    executable: bhap.exe
    zip: false
  webgl:
    path: dist/webgl

BrettCleary

Going to start testing now 👌

BrettCleary · 2024-09-11T22:35:31Z

src/commands/publish.ts

@@ -70,7 +70,7 @@ export default class Publish extends Command {
    if (!config.platforms) this.error('no platforms configured');

    for (const [key, value] of Object.entries(config.platforms)) {
-      if (!value.executable) this.error(`No executable path found for platform ${key}`)
+      if (!value.executable && key !== "webgl") this.error(`No executable path found for platform ${key}`)


we need to be able to support web as well in the platform keys in the hyperplay.yml file

i.e. a hyperplay.yml file like

account: game_dev_account project: game release: 0.1.0 description: Release notes go here. platforms: webgl: path: dist/webgl/ zip: false web: external_url: https://hyperplay.xyz windows_amd64: path: dist/windows/amd64/ zip: true executable: Game\\Game.exe

might make sense to add an override variable in the platform config as well like ignoreExecutableCheck for a platform so this check is opt-out and a general solution

Re: my below message, I think ignoreExecutableCheck is a good to have but if it's not expected for a web or a webgl build, I don't believe the user should have to add it. I worry that the config is already getting a bit verbose, would a user have to do?

webgl: path: dist/webgl ignoreExecutableCheck: false isFolderUpload: false

Also as another thought, it may be bad to enable certain things as general solutions, for all of the native fields we want to enforce the executable, and it would actually cause a lot of problems for them to skip the check.

src/commands/publish.ts

BrettCleary · 2024-09-11T22:51:03Z

src/releases.ts

+    const isWebGL = platformKey === 'webgl';
+    const files = isWebGL ? await getFolderFiles(platformPath) : await getSingleFile(platformPath);


We should try to avoid adding if statements throughout the code for various platform key specific scenarios. It keeps us from making more general interfaces and the code is harder to read/more complicated

Could we add a field on the config hyperplay.yml file for isFolderUpload or something like that?

Then we would get its value here like

const isFolderUpload = config.platforms[platformKey].isFolderUpload const files = isFolderUpload ? await getFolderFiles(platformPath) : await getSingleFile(platformPath);

I generally agree, though I still think it's a balance between clean code and developer experience, realistically we only are going to support a very small set of platforms for the foreseeable future, I think it's way more worth it to provide as minimal a configuration and as good of a developer experience before purity of interfaces, or future state code complexity.

I think isFolderUpload is a great idea but if the intended purpose of the webgl builds is for it to upload a folder of webgl assets it seems like it only creates more friction to make the publisher configure additional fields.

Either way it's a good to have config, just wanted to express some thought's around dev ux vs code purity.

BrettCleary · 2024-09-11T22:53:33Z

src/releases.ts

+    CliUx.ux.action.stop();
+
+    for (const url of urls) {
+      const fileData = isWebGL ? files.find(f => f.fileName === url.fileName)?.filePath : platformPath;


we could replace isWebGL by isFolderUpload here too

BrettCleary · 2024-09-11T23:36:33Z

Uploaded phantom galaxies and it worked!

the logs were

Successfully uploaded files to HyperPlay: https://gateway-b3.valist.io/hyperplaycd/irontest/v0.2.0
Publishing release... done
Confirming transaction 0xbc54832b9a2a230a17edf3d0eefa9279c54abbbe7f8cd8ce1ae3ca7cfc43e29a... done
Successfully published hyperplaycd/irontest/v0.2.0!
view the release at:

could we remove the "view the release at:" part since no url is shown?

this url is also not working so should we remove that from the logs? https://gateway-b3.valist.io/hyperplaycd/irontest/v0.2.0 @jiyuu-jin

BrettCleary · 2024-09-11T23:45:19Z

When uploading using this hyperplay.yml file

account: hyperplaycd
project: irontest
release: v0.2.1

platforms:
  windows_amd64: 
    path: ./windows_amd64.zip
    zip: false
    executable: test_win_x64.txt
  web: 
    external_url: https://hyperplay.xyz
    zip: false
  webgl:
    path: ./test/mock_data/mac_arm64/test_mac_arm64.txt
    zip: false

I get

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of Buffer or URL. Received undefined
    at Object.stat (node:internal/fs/promises:893:10)

because path is not set for "web" but web platform should not have a path.

BrettCleary · 2024-09-11T23:47:39Z

uploading an uncompressed folder for webgl seemed to work with

account: hyperplaycd
project: irontest
release: v0.2.1

platforms:
  windows_amd64: 
    path: ./windows_amd64.zip
    zip: false
    executable: test_win_x64.txt
  webgl:
    path: ./test/mock_data/mac_arm64
    zip: false

Successfully uploaded files to HyperPlay: https://gateway-b3.valist.io/hyperplaycd/irontest/v0.2.1
Publishing release... done
Confirming transaction 0xc7409dd140b114c7edb3515ee74f331aba94bf2b7abad6ea19ac1c2885c21ec3... done
Successfully published hyperplaycd/irontest/v0.2.1!
view the release at:

Co-authored-by: Brett <[email protected]>

jiyuu-jin · 2024-09-12T14:44:59Z

When uploading using this hyperplay.yml file

account: hyperplaycd
project: irontest
release: v0.2.1

platforms:
  windows_amd64: 
    path: ./windows_amd64.zip
    zip: false
    executable: test_win_x64.txt
  web: 
    external_url: https://hyperplay.xyz
    zip: false
  webgl:
    path: ./test/mock_data/mac_arm64/test_mac_arm64.txt
    zip: false

I get

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of Buffer or URL. Received undefined
    at Object.stat (node:internal/fs/promises:893:10)

because path is not set for "web" but web platform should not have a path.

Really not trying to be contrarian but I have similar feedback to this as above, I think it's quicker, and better Dev UX in the short term to just provide reasonable defaults for each platform that require the minimal amount of config.

I just don't think it makes sense for the user to have to put things like zip: false when external_url is present it's just confusing when it's clearly an external uri.

I also think it's a lot simpler to just do something like below , where we manually check if it's a web platform with an externalUrl and just skip the rest of the logic. A user should not have to set zip: false or any of the other values other than externalUrl.

Conversely I think we'd want to lock down the externalUrl field to just the web platforms because native builds are generally much larger than web files we don't want native builds to be able to specify their own external path because the games files won't be behind our cdn and cause a bad player experience.

    if (externalUrl && platformKey === "web") {
      meta.platforms[platformKey] = {
        name: "web",
        external_url: externalUrl,
      };
      continue;
    }

BrettCleary · 2024-09-12T21:07:40Z

I just don't think it makes sense for the user to have to put things like zip: false when external_url is present it's just confusing when it's clearly an external uri.

I agree with this statement

BrettCleary · 2024-09-12T21:22:09Z

@jiyuu-jin

This yml file

account: hyperplaycd
project: irontest
release: v0.2.2

platforms:
  windows_amd64: 
    path: ./windows_amd64.zip
    zip: false
    executable: test_win_x64.txt
  web: 
    external_url: https://hyperplay.xyz
    zip: false
  webgl:
    path: ./test/mock_data/mac_arm64/test_mac_arm64.txt
    zip: false

is giving this error

Generating presigned URLs for windows_amd64... done
Upload progress for windows_amd64 - windows_amd64.zip: 100%
Upload progress for windows_amd64 - windows_amd64.zip: 100%
    TypeError: The "path" argument must be of type string or an instance of Buffer or URL. Received undefined
    Code: ERR_INVALID_ARG_TYPE

BrettCleary · 2024-09-12T21:23:23Z

@jiyuu-jin I tried then adding the path for the web platform to see if that would fix

This yml file

account: hyperplaycd
project: irontest
release: v0.2.2

platforms:
  windows_amd64: 
    path: ./windows_amd64.zip
    zip: false
    executable: test_win_x64.txt
  web: 
    path: ./test
    external_url: https://hyperplay.xyz
    zip: false
  webgl:
    path: ./test/mock_data/mac_arm64/test_mac_arm64.txt
    zip: false

is giving this error

Upload progress for windows_amd64 - windows_amd64.zip: 100%
Upload progress for windows_amd64 - windows_amd64.zip: 100%
Generating presigned URLs for web... done
Error completing multipart upload AxiosError: Request failed with status code 500

jiyuu-jin added 7 commits September 6, 2024 18:22

Bump package version

faa49c0

Test new presigned url with single file

c0ddd3a

Add WebGL folder uploads

7619f2e

Add tests for new upload logic/endpoints

e9c6bd4

Add dynamic mime-type detection for files/folders uploaded to s3

e8855cb

Bump cli package version and bump @valist/sdk to 2.10.4

f4b4ab4

Add fileName to progress logs

4794124

jiyuu-jin added the enhancement New feature or request label Sep 11, 2024

jiyuu-jin self-assigned this Sep 11, 2024

jiyuu-jin requested review from flavioislima, BrettCleary and eliobricenov September 11, 2024 20:59

BrettCleary reviewed Sep 11, 2024

View reviewed changes

jiyuu-jin and others added 2 commits September 11, 2024 23:04

Update src/commands/publish.ts

ff05d68

Co-authored-by: Brett <[email protected]>

Add support for web platform externalUrl

7285b11

Fix missing externalUrl

26ebb3a

BrettCleary self-requested a review September 12, 2024 22:52

BrettCleary approved these changes Sep 12, 2024

View reviewed changes

Add @todo comments and location dirname trim for WebGL platform

33d503c

BrettCleary merged commit 7188a0e into main Sep 13, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat] Add folder uploading #33

[Feat] Add folder uploading #33

jiyuu-jin commented Sep 9, 2024 •

edited

Loading

BrettCleary left a comment

BrettCleary Sep 11, 2024

BrettCleary Sep 11, 2024

jiyuu-jin Sep 12, 2024

BrettCleary Sep 11, 2024

jiyuu-jin Sep 12, 2024 •

edited

Loading

BrettCleary Sep 11, 2024

BrettCleary commented Sep 11, 2024

BrettCleary commented Sep 11, 2024

BrettCleary commented Sep 11, 2024

jiyuu-jin commented Sep 12, 2024 •

edited

Loading

BrettCleary commented Sep 12, 2024 •

edited

Loading

BrettCleary commented Sep 12, 2024 •

edited

Loading

BrettCleary commented Sep 12, 2024

		const isWebGL = platformKey === 'webgl';
		const files = isWebGL ? await getFolderFiles(platformPath) : await getSingleFile(platformPath);

[Feat] Add folder uploading #33

[Feat] Add folder uploading #33

Conversation

jiyuu-jin commented Sep 9, 2024 • edited Loading

Description

Testing

BrettCleary left a comment

Choose a reason for hiding this comment

BrettCleary Sep 11, 2024

Choose a reason for hiding this comment

BrettCleary Sep 11, 2024

Choose a reason for hiding this comment

jiyuu-jin Sep 12, 2024

Choose a reason for hiding this comment

BrettCleary Sep 11, 2024

Choose a reason for hiding this comment

jiyuu-jin Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

BrettCleary Sep 11, 2024

Choose a reason for hiding this comment

BrettCleary commented Sep 11, 2024

BrettCleary commented Sep 11, 2024

BrettCleary commented Sep 11, 2024

jiyuu-jin commented Sep 12, 2024 • edited Loading

BrettCleary commented Sep 12, 2024 • edited Loading

BrettCleary commented Sep 12, 2024 • edited Loading

BrettCleary commented Sep 12, 2024

jiyuu-jin commented Sep 9, 2024 •

edited

Loading

jiyuu-jin Sep 12, 2024 •

edited

Loading

jiyuu-jin commented Sep 12, 2024 •

edited

Loading

BrettCleary commented Sep 12, 2024 •

edited

Loading

BrettCleary commented Sep 12, 2024 •

edited

Loading