PrismarineJS / prismarine-realms

Library for interacting with Minecraft Realms
MIT License
11 stars 9 forks source link

Initial implementation #1

Closed LucienHH closed 2 years ago

extremeheat commented 2 years ago

Ideally we should have a well-defined API here instead of being a unstable wrapper around fetch(). Some possibilties:

interface RealmAPI {
  isAvaliable(checkCompatible=true): Promise<boolean> // check if can access realms
  getWorlds(): Promise<World[]>
  getWorld(id): Promise<World>
}

interface World {
  getAddress(): Promise<hostname and port>
  getBackups(): Backup object
  getDownloadURL(): Promise<string>
  downloadBackup(dir): Promise
  getOperators(): Promise<string[]>
  // ...
}

We don't need to implement the entire thing in one go, but creating an API like above ^ will make it expandable in the future to more parts of their API

LucienHH commented 2 years ago

Ok, will work on this now.

extremeheat commented 2 years ago

Here is an idea on implementation design: https://gist.github.com/extremeheat/5adc445437d91c466119f9b70640931a

LucienHH commented 2 years ago

Thanks for the example it's been useful. The problem I'm trying to figure out now is adding routes that take different inputs for either version without creating separate Realm classes for each API, for example say we added invitePlayer to the Realm class:

invitePlayer() {
  return this.api.realmInvitePlayer('...')
}  

For bedrock we just need a uuid invitePlayer(uuid: string) but for java, it's uuid and name invitePlayer(uuid: string, name: string). Do we not mind that it has a useless input if using the Bedrock API or do we need to think of a solution when both APIs need separate inputs. I hope I explained this well enough so it's understandable.

extremeheat commented 2 years ago

Yes extra parameters and options are fine, still API compatible

extremeheat commented 2 years ago

No major problems outside the last one, other than addressing rom's comments 👍

extremeheat commented 2 years ago

any updates on this @LucienHH ?

LucienHH commented 2 years ago

Yeah, been away for a couple days. Will add some changes later tonight.

extremeheat commented 2 years ago

No major concerns from me, can you add some tests here? Even basic ones with simulated API is ok. A manual test could be added on top like we have for prismarine auth

LucienHH commented 2 years ago

Tests are something I'm working on. I plan to use nock to test the API and we could have a manual test to check authentication, hosts etc.

LucienHH commented 2 years ago

Tests added, will come back to add manual tests in the future.

LucienHH commented 2 years ago

Is this OK to merge @rom1504 or are there any other changes needed?

extremeheat commented 2 years ago

Can you implement in protocol library? You can add the dependency pointed as a git dep for now so tests can run in them

rom1504 commented 2 years ago

merged

follow up:

rom1504 commented 2 years ago

if you can continue on this @LucienHH it will be great