webarkit / ARnft

A small javascript library for WebAR with NFT
GNU Lesser General Public License v3.0
219 stars 53 forks source link

ARnft code design: export as a encapsulated class? #144

Closed kalwalt closed 2 years ago

kalwalt commented 3 years ago

At the moment ARnft is exported inside index.js:

https://github.com/webarkit/ARnft/blob/69207041bbacd2fd19f67599e334fc9bb23bb88b/src/index.js#L1-L5

In our standard example after "importing" the dist library:

https://github.com/webarkit/ARnft/blob/69207041bbacd2fd19f67599e334fc9bb23bb88b/examples/arNFT_example.html#L19

we can use in our script as:

https://github.com/webarkit/ARnft/blob/69207041bbacd2fd19f67599e334fc9bb23bb88b/examples/arNFT_example.html#L22

Does it is correct? Should be better and conforable start ARnft in this way:

ARnft.init(640, 480, "examples/DataNFT/pinball", 'config.json', true)

instead?

importing it into React instead you do:

import { ARnft } from '@kalwalt/ar-nft'

I think if we change our index.js:

// inside index.js

import Arnft from './ARnft'

after we can:


ARnft.init(640, 480, "examples/DataNFT/pinball", 'config.json', true)```
hoaile175 commented 3 years ago

Hi, I don't use React. How do I get started with the available examples. I tried checking out source (v0.8.4) and running with local host; When running on phone (iOS), it stuck with loading screen. It does not show up for the notice of camera access.

albjeremias commented 2 years ago

I think there are many issues on this code. https://github.com/webarkit/ARnft/blob/master/examples/arNFT_multi_example.html#L28

const entities = [
            { name: 'pinball', markerUrl: "examples/DataNFT/pinball" },
            { name: 'kuva', markerUrl: "examples/DataNFT/kuva" },
            { name: 'chalk_multi', markerUrl: "examples/DataNFT/chalk_multi"}
        ]

ARnft.ARnft.initWithEntities(640, 480, entities, 'config.json', true)

First config.json needs to be in a path, that always creates issues, would be better to load the config.json and then pass an object as a argument. I don't understand why entities are not included in the config.json makes it really confusing. showing the stats and the logo should be false by default.

I'd go for something more like:

const configs = require('./config.json');
ARnft.setSize(640,480);
ARnft.init(configs);

I get really confused with 2 places to setup the size.. one is in the init, the other in the config.json. what are the differences between them?

{
  "addPath": "",
  "cameraPara": "examples/Data/camera_para.dat",
  "container": {
    "create": true,
    "containerName": "",
    "canvasName": ""
  },
  "videoSettings": {
    "width": {
      "min": 640,
      "max": 800
    },
    "height": {
      "min": 480,
      "max": 600
    },
    "facingMode": "environment"
  },
  "loading": {
    "create": true,
    "logo": {
      "src": "Data/arNFT-logo.gif",
      "alt": "arNFT.js logo"
    },
    "loadingMessage": "Loading, please wait..."
  },
  "stats": {
    "createHtml": true
  }
}
kalwalt commented 2 years ago

@albjeremias i understand you, sometimes it can be confusing. We could create only a init() function without a initWithEntities(), simplifyng the init making some default settings in a internal config. Entities could be added instad with an addEntity function and attached to ARnft. I will open anothe rissue for this also because we are a bit off topic :smile: in regards of the thread.

kalwalt commented 2 years ago

Back to the original topic: i think we don't need to change anything.