AR-js-org / AR.js

Image tracking, Location Based AR, Marker tracking. All on the Web.
MIT License
5.3k stars 909 forks source link

Testing type definitions (Typescript) #506

Closed kalwalt closed 9 months ago

kalwalt commented 1 year ago

⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️

What kind of change does this PR introduce?

This PR will try to add Type definitions to be used in a Typescript project.

Can it be referenced to an Issue? If so what is the issue # ? see #402

How can we test it?

No test provided yet, but i will create a Typescript project for testing. Summary

Does this PR introduce a breaking change? It shouldn't create any issue.

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser I'm testing types defs in this my repository https://github.com/kalwalt/AR.js-typescript

Other information

commentatorboy commented 1 year ago

Hey. So I tried to implement this on my vue project https://github.com/commentatorboy/ARJSVue

As mentioned previously when installing the package, and setting up the project it will only work if you click the "VR" button on the video.

To test this just do the following:

  1. npm install
  2. npm run dev
  3. Go to localhost and go into the "MyARProjects" page.
  4. You will be promted to use the camera
  5. Accept and then scan the hiro marker.
  6. It does not show up.
  7. Now there should be a "vr" button or a "full screen" button where it does not show the camera as background, but a black screen.
  8. Scanning the hiro marker then shows the T-Rex.

You can see what I am scanning with my webcam. (Also notice the "fullscreen" button at the lower right corner) image

Pressing that "fullscreen" button shows the T rex image

Btw. I have not done anything other than using the Vue quick start guide. https://vuejs.org/guide/quick-start.html#creating-a-vue-application And then adding a new page that has hiro marker example: https://github.com/AR-js-org/AR.js#-marker-based-example

kalwalt commented 1 year ago

Hey. So I tried to implement this on my vue project https://github.com/commentatorboy/ARJSVue

As mentioned previously when installing the package, and setting up the project it will only work if you click the "VR" button on the video.

To test this just do the following:

  1. npm install
  2. npm run dev
  3. Go to localhost and go into the "MyARProjects" page.
  4. You will be promted to use the camera
  5. Accept and then scan the hiro marker.
  6. It does not show up.
  7. Now there should be a "vr" button or a "full screen" button where it does not show the camera as background, but a black screen.
  8. Scanning the hiro marker then shows the T-Rex.

You can see what I am scanning with my webcam. (Also notice the "fullscreen" button at the lower right corner) image

Pressing that "fullscreen" button shows the T rex image

Btw. I have not done anything other than using the Vue quick start guide. https://vuejs.org/guide/quick-start.html#creating-a-vue-application And then adding a new page that has hiro marker example: https://github.com/AR-js-org/AR.js#-marker-based-example

@commentatorboy i forgot to add types entry to the package.json file, i will add soon as i can.

kalwalt commented 1 year ago

I'm testing type definitons in this my repository: https://github.com/kalwalt/AR.js-typescript This is what i found:

import ArMultiMarkerControls from "./markers-area/arjs-markersareacontrols";
import ArMultiMakersLearning from "./markers-area/arjs-markersarealearning";
import ArMultiMarkerUtils from "./markers-area/arjs-markersareautils";
export {
  ArMarkerControls,
  ArMarkerHelper,
  ArSmoothedControls,
  ArToolkitContext,
  ArToolkitProfile,
  ArToolkitSource,
  ArMultiMarkerControls,
  ArMultiMakersLearning,
  ArMultiMarkerUtils,
};
//# sourceMappingURL=index-threex.d.ts.map

but should be instead:

import ArMarkerControls from "./threex/arjs-markercontrols";
import ArMarkerHelper from "./threex/threex-armarkerhelper";
import ArSmoothedControls from "./threex/threex-arsmoothedcontrols";
import ArToolkitContext from "./threex/arjs-context";
import ArToolkitProfile from "./threex/arjs-profile";
import ArToolkitSource from "./threex/arjs-source";
import ArMultiMarkerControls from "./markers-area/arjs-markersareacontrols";
import ArMultiMakersLearning from "./markers-area/arjs-markersarealearning";
import ArMultiMarkerUtils from "./markers-area/arjs-markersareautils";
export {
  ArMarkerControls,
  ArMarkerHelper,
  ArSmoothedControls,
  ArToolkitContext,
  ArToolkitProfile,
  ArToolkitSource,
  ArMultiMarkerControls,
  ArMultiMakersLearning,
  ArMultiMarkerUtils,
};
//# sourceMappingURL=index-threex.d.ts.map

not all type definitions are created by the tsc compiler, don't know why...

commentatorboy commented 1 year ago

Now I am not entirely into typescript, so I might be not be of too much help here 😓

But I have some questions though Why should it be import ArMarkerControls from "./threex/arjs-markercontrols";

It is not exported in https://github.com/AR-js-org/AR.js/blob/master/three.js/src/threex/arjs-markercontrols.js It is just called MarkerControls.

Where is this "mapping" defined?

I agree on point 4. It would make sense to start with upgrading the code to Typescript, to at least be able to make it more modular.

kalwalt commented 1 year ago

Now I am not entirely into typescript, so I might be not be of too much help here

I'm not a Typescript guruu but i think i can develop a Typescript version with help of the AR.js community!

But I have some questions though Why should it be import ArMarkerControls from "./threex/arjs-markercontrols";

It is not exported in https://github.com/AR-js-org/AR.js/blob/master/three.js/src/threex/arjs-markercontrols.js It is just called MarkerControls.

Where is this "mapping" defined?

This because the old implementation doesn't adopt the new ES6 syntax when it was developed ( that come later... take a look at the old code here: https://github.com/AR-js-org/AR.js/blob/aaad9847f67a4738b00724c38d9f501d78e0a8af/three.js/src/threex/threex-armarkercontrols.js

var ARjs = ARjs || {}
var THREEx = THREEx || {}

ARjs.MarkerControls = THREEx.ArMarkerControls = function(context, object3d, parameters){
    var _this = this
        // other code here...
}

I think we should go over this and think in a more modern approach. As i said, and i remark this, we should start to redesign the code with a modular concept in mind. In 2023 we all the tools for doing this. We could start with a new AR.js-threex and AR.js-base modules, that can be integrated into AR.js.

kalwalt commented 1 year ago

And more: With Typescript we can create two namespaces AR.js and THREEx and create inside the two, all the classes.


// inside ArMarkerControls.ts

import { IContext, IObject3d, IParams } from 'interfaces/CommonInterfaces'

namespace THREEx {

  class ArMarkerControls {
    constructor(context: IContext, object3d: IObject3D, parameters: IParams) {

    }
  }
}

// inside MarkerControls.ts
import ArMarkerControls from '@ar-js-org/arjs-threex'

namespace ARjs {

  class MarkerControls extends ArMarkerControls {
    constructor(context: any, object3d: any, parameters: any) {
      super(context, object3d, parameters)
    }
  }
}
commentatorboy commented 1 year ago

I think we should go over this and think in a more modern approach. As i said, and i remark this, we should start to redesign the code with a modular concept in mind

Agreed. If we start with AR.js-threex and AR.js-base Then I think we should have a repo and layout the files that we can work on :D And also what exactly the tasks are. I and try and help with converting one of the files to ES6. And then we can continue from there 👍

kalwalt commented 1 year ago

Working on a typescript version this my repo ARj.s-threex. There is a basic example, but not fully working yet. I hope to have more time next week to improve the code.

kalwalt commented 1 year ago

In regards of this PR i think i will not merge. I think there are many issues in it to be solved.

nickw1 commented 1 year ago

@kalwalt sounds the best plan if so.

kalwalt commented 9 months ago

@commentatorboy have you tried AR.js-threejs? Closing this PR because as i said there are too many issues. I will think how we can add typedefintions to Ar.js before we can refactor the code with a more modern approach.