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

WIP: issue #213 - load chunks from a dat, prototype #214

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ryanramage
Copy link
Collaborator

WIP: Dont merge :)

Ok, this is pretty hacky but just wanted to try it out. Some notes.

  • I randomly picked a ui thing called smalltalk just so I could get a prompt for a dat url. This can be dropped, but I will need help making a better ui
  • I just prototyped a basic externalDescriptor. Just kind of copied examples I saw. I may be missing things, and we could in the future have the dat have some meta data that can be merged into this descriptor
  • Not much error handling at the moment
  • The dat that is loaded is expected to have a index.json file. We could move to a package.json?

The dat I have right now for testing is: dat://ac43fde5530e1929d0740cfdd36d5790d9f489660f7db934ebc3ec075ea56342

it is currently mirrored at https://firepit-ryanramage.hashbase.io

Adding the chunk to the grid fails because of the originalDirectory seems to mess up nodes/setup/object.js importChunk method.

Feedback would be very welcome!

@mmckegg
Copy link
Owner

mmckegg commented Sep 14, 2017

Awesome!! I remember trying this with WebTorrent years ago. Dat makes much more sense 😄

Will look at this in detail and provide a review later on (hopefully today)!

@mmckegg
Copy link
Owner

mmckegg commented Sep 14, 2017

@ryanramage Are you imagining that the chunk would update when that dat changes? Is this supposed to sync?

@ryanramage
Copy link
Collaborator Author

I dont imagine sync for chunks once loaded. The chunk author could move on with changes, but for chunks downloaded it won't update.

Later I want to move onto setups and project level dats, and there sync will make more sense.

@mmckegg
Copy link
Owner

mmckegg commented Sep 14, 2017

Awesome, just tried this out!

Adding the chunk to the grid fails because of the originalDirectory seems to mess up nodes/setup/object.js importChunk method.

Yeah, I think this is because Loop Drop isn't expecting a chunk to be in a sub-directory. Obviously this should work, but it is running up against a hack I'm using to detect that chunk should be imported from another setup.

On this line: https://github.com/mmckegg/loop-drop-app/blob/master/nodes/loop-grid/grid.js#L278 there is a check if the directory matches, but it is checking absolutely rather than a subset. I think if this check was updated to check of the subtree matched, everything would just work. There might be some other issues introduced when dragging one of these dat chunks into another setup though for similar reasons.

This is a great example of why Loop Drop needs some tests 😆

I had so much trouble trying to create web audio tests, that I just kind of gave up on them. But this kind of thing would be so easy to write automated tests for. Derp.

@ryanramage
Copy link
Collaborator Author

The other side of this will be to publish chunks, which would be nice if new chunks got put in a subdirectory with the index.json as the standard.

@@ -17,6 +18,7 @@ module.exports = function (collection, nodeName, index, cb) {
var lookup = context.nodeInfo.lookup
var info = lookup[nodeName]
if (info) {
if (info.name === 'dat') return createDatNode(collection, index, context, fileObject, cb)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet hack!

I think something like this could be made official by making it a little more generic. It could be something more like this:

if (typeof info.action === 'function') return info.action({collection, index, context, fileObject}, cb)

And then you would move createDatNode into nodes/dat-chunk/index.js.

var createDatNode = require('./create-dat-node')
module.exports = {
  name: 'dat',
  node: 'dat-chunk',
  action: (opts, cb) => {
    createDatNode(opts.collection, opts.index, opts.context, opts.fileObject, cb)
  },
  group: 'simpleChunks',
  description: 'Load a chunk from dat p2p'
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, That's a helpful pointer. I will adjust the code it to that.

@mmckegg
Copy link
Owner

mmckegg commented Sep 14, 2017

new chunks got put in a subdirectory with the index.json as the standard

Yeah, this is probably a good idea.

@mmckegg
Copy link
Owner

mmckegg commented Sep 14, 2017

@ryanramage If it's okay with you, adding you as a collaborator! Feel free to work on this in a branch inside mmckegg/loop-drop-app.

@ryanramage
Copy link
Collaborator Author

Thanks for adding me as a collaborator. I am out of steam for today but I will do some more work tomorrow.

mmckegg added a commit that referenced this pull request Sep 14, 2017
@mmckegg
Copy link
Owner

mmckegg commented Sep 14, 2017

Adding the chunk to the grid fails because of the originalDirectory seems to mess up nodes/setup/object.js importChunk method.

Okay fixed this in 09b1a85

If you rebase with the latest master, the dat chunk now can be added to a grid!

@ryanramage
Copy link
Collaborator Author

Thanks, this I have merged master, and I have updated based on your review. The dat is working.

I have created a larger chunk a massive 64 sampled drum kit, which can be found here dat://e9569d6ff35812c3269e55b3be144512ac6bad1f3ec8510fd389afd0fdcd202a

I am not sure why the grid on drop only shows a 1x4 rectangle. I have to go in and edit the chunk to tell it that it is 8x8. What would I need to change to make this happen?

@ryanramage
Copy link
Collaborator Author

Last improvement I would want to make would be a better 'prompt' - Any pointers on how I might do that?

@mmckegg
Copy link
Owner

mmckegg commented Sep 19, 2017

@ryanramage

I am not sure why the grid on drop only shows a 1x4 rectangle. I have to go in and edit the chunk to tell it that it is 8x8. What would I need to change to make this happen?

I think this is because the size of a chunk is stored in the setup rather than the external chunk. You might need to include some additional meta data to be applied directly to the chunk in the export file.

These are the params that are stored directly in the setup for external chunks:

{
    id: "drums 1",
    shape: [ 3, 4 ],
    flags: [  ],
    chokeAll: false,
    chokeGroup: null,
    color: [ 251, 141, 197 ],
    minimised: true,
    src: "./drums 1.json",
    offset: 0,
    routes: { output: "drums 1-output#input" },
    paramValues: { dist: 0, delayinfit: 0 },
    volume: 1.4428,
    node: "externalChunk"
  }

Last improvement I would want to make would be a better 'prompt' - Any pointers on how I might do that?

Good question. I haven't added any modal dialogs into Loop Drop so far, so there isn't really anything to repurpose. I will give this some more thought.

I've been wondering if the button should be "Import" rather than "dat", to make it a bit more generic.

Have you thought much about how the interface should work for exporting a chunk to dat?

fetchDat(datHash, folderPath, function (err) {
var path = fileObject.resolvePath(datHash + '/index.json')
var externalDescriptor = {
node: 'Node 1',
Copy link
Owner

@mmckegg mmckegg Sep 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned in this comment, you might want to include some of these params (particularly shape) in the export itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thats a good idea. I need to start thinking of how to add meta to the chunk so publishers can help people find them. I think for the exported chunk, I will add a property to the index.json called "published". So it can contain published metadata that can be used by the externalDescriptor

so the file might look like this at the top
e9569d6ff35812c3269e55b3be144512ac6bad1f3ec8510fd389afd0fdcd202a/index.json

{
  "published": {
    "name": "large drumkit",
    "publisher": "Ryan Ramage",
    "tags": ["drumkit", "samples"], 
    "shape": [8, 8]
    "color": [251, 141, 197],
    "paramValues": { "dist": 0, "delayinfit": 0 },
    "volume": 1.4428
  },
  slots: {
    ...

Later we can start indexing this info into a db and make a search that can help people find stuff.

I think I want to add a 'publish' button to the tab that appears when you edit a chunk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants