Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loadNFTMarkers has incorrect onSuccess signature #230

Closed
antokhio opened this issue Nov 16, 2022 · 5 comments
Closed

loadNFTMarkers has incorrect onSuccess signature #230

antokhio opened this issue Nov 16, 2022 · 5 comments
Assignees
Labels
bug Something isn't working Typescript

Comments

@antokhio
Copy link

loadNFTMarkers has incorrect return signature

(method) ARControllerNFT.loadNFTMarkers(urlOrData: string[], onSuccess: (ids: number) => void, onError: () => void): Promise<[{
    id: number;
}]>
onSuccess: (ids: number) should be onSuccess: (ids: number[])

currently have to convert it to unknow first:

await controller?.loadNFTMarkers(markers, (ids:unknown) => {...}, ... )
@antokhio antokhio changed the title loadNFTMarkers has incorrect return signature loadNFTMarkers has incorrect onSuccess signature Nov 16, 2022
@kalwalt
Copy link
Member

kalwalt commented Nov 16, 2022

Hi @antokhio have you a piece of code to test? currently with ARnft (see Worker.ts) i never had this issue https://github.com/webarkit/ARnft/blob/bf748f29af0ce694c2f8730115468a113fb884a4/src/Worker.ts#L104-L116

      ar.loadNFTMarkers(nftMarkerUrls, (id: number) => {
           let marker = ar.getNFTData(ar.id, 0);
           ctx.postMessage({ type: "markerInfos", marker: marker });
           ar.trackNFTMarkerId(id);
           console.log("loadNFTMarker -> ", id);
           console.log(id);
           ctx.postMessage({ type: "endLoading", end: true });
       }).catch((err: any) => {
           console.error("Error in loading marker on Worker", err);
       });


       ctx.postMessage({ type: "loaded", proj: JSON.stringify(cameraMatrix) });
   };

I think jsartoolkitNFT need some improves in type checking see for example the LoadNFTMarkers

async loadNFTMarkers(
urlOrData: Array<string>,
onSuccess: (ids: number) => void,
onError: () => void
) {
let nft = await this.artoolkitNFT.addNFTMarkers(
this.id,
urlOrData,
(ids: any) => {
this.nftMarkerCount += ids.length;
onSuccess(ids);
},
onError
);
return nft;
}

at line 712

(ids: any) => {
...
}

I think this is really odd...

@kalwalt kalwalt self-assigned this Nov 16, 2022
@kalwalt kalwalt added bug Something isn't working Typescript labels Nov 16, 2022
@antokhio
Copy link
Author

@kalwalt here is WIP project, https://github.com/antokhio/react-three-jsartoolkitnft/blob/main/src/workers/arnft.worker.ts

Just to note, trying to make it an npm package, but i have issues with web worker bundling i can't sort out for a week or so ;)
The code should be working if you copy the worker js from npm package to public.

if you wanna test it, this minimum code to make it work:

function App() {
  return (
    <ARNFTCanvas>
      <ARNFTMarker markers={['/markers/pinball']} persisted>
        <Box args={[100, 100, 100]} />
      </ARNFTMarker>
      <ARNFTMarker markers={['/markers/insta']}>
        <Sphere args={[100, 100, 100]} />
      </ARNFTMarker>
    </ARNFTCanvas>
  );
}

p.s. there gonna be a bit of refactor/cleanup when i manage to make worker portable

@antokhio
Copy link
Author

antokhio commented Nov 16, 2022

@kalwalt the problem is line 706

onSuccess: (ids: number) => void,

this prevents call with correct signature....
likely it's just misstyped

however according to this:

addNFTMarkers: (
arId: number,
urls: Array<string>,
callback: (filename: any) => void,
onError2: (errorNumber: any) => void
) => [{ id: number }];

the return type should be:

{id: number}[]

not sure, looks like an error in both places actually
according to my code it returns:

{
   id: number;
   width: number;
   height: number;
   dpi: number;
}[]

@kalwalt
Copy link
Member

kalwalt commented Nov 17, 2022

@antokhio i will try to fix in a PR i hope soon.

@kalwalt
Copy link
Member

kalwalt commented Dec 1, 2022

with the latest in PR #231 it should works in this way:

await ar?.loadNFTMarkers(nftMarkerUrls, (id: number[]) => {
      let marker = ar.getNFTData(id[i], 0);
      ctx.postMessage({ type: "markerInfos", marker: marker });
      ar.trackNFTMarkerId(id[i]);
      console.log("loadNFTMarker -> ", id);
      console.log(id);
      ctx.postMessage({ type: "endLoading", end: true });
      }, () => {}).catch((err: string) => {
          console.error("Error in loading marker on Worker", err);
      });

@kalwalt kalwalt closed this as completed in e8a3cec Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Typescript
Projects
None yet
Development

No branches or pull requests

2 participants