piskel icon indicating copy to clipboard operation
piskel copied to clipboard

fix(build): update deps, karma and nw config

Open ayushmanchhabra opened this issue 3 years ago • 8 comments

Fixes: #973, #1047 Closes: #1013, #1041

Changes

  • Allow installation of node_modules on Node 14 and above 11e0066
  • Switch to Chromium Headless with Karma to successfully run tests 9bbc9d2
  • ~Use a fork of grunt nw plugin to get desktop builds working bb4c4fe~
  • Replace build and build:mac commands with desktop:run and desktop:build (for all platforms except osx32).

Build Instructions

  • Using your package manager of choice, desktop:run to run the desktop app directly.
  • desktop:build to build applications for all platforms except osx32.

I'll be making a bunch of fixes in this branch and release them at https://ayushmxn.github.io/piskel

Things yet to be done

  • Update IndexedDB implementation to get tests passing again
  • Rewrite CLI using yargs
  • Update code until possible to remove Phantom dependency

If you're able to, I'd urge you to use Aseprite instead.

ayushmanchhabra avatar Feb 11 '22 12:02 ayushmanchhabra

Thanks for your work on this!

The build system now works.

However, the current build for macOS is broken due to problems with old NW.js on latest macOS. So I needed to make one more change, to increase the version of nwjs:

macos : {
  options: {
    downloadUrl: 'https://dl.nwjs.io/',
    osx64: true,
    version : "0.63.1",
    build_dir: './dest/desktop/',
    flavor: "normal",
  },
  src: ['./dest/prod/**/*', "./package.json", "!./dest/desktop/"]
},

gingerbeardman avatar Apr 28 '22 12:04 gingerbeardman

Thanks for your work on this!

The build system now works.

However, the current build for macOS is broken due to problems with old NW.js on latest macOS. So I needed to make one more change, to increase the version of nwjs:

macos : {
  options: {
    downloadUrl: 'https://dl.nwjs.io/',
    osx64: true,
    version : "0.63.1",
    build_dir: './dest/desktop/',
    flavor: "normal",
  },
  src: ['./dest/prod/**/*', "./package.json", "!./dest/desktop/"]
},

Good to know! I don't use Macs which is probably why I missed this. I'll make the change later today.

ayushmanchhabra avatar Apr 30 '22 00:04 ayushmanchhabra

I'd like to make some changes for quality of life improvements, such as a preference to switch off warning messages and different defaults depending in the filename.

I am familiar with JavaScript, but what is the main thing I will need to read up on to be able to edit work with this codebase?

gingerbeardman avatar Apr 30 '22 18:04 gingerbeardman

I'm not sure what could act as a good reference. From what I've seen, the codebase is pretty old - there's a polyfill for Promises and the underlying browser APIs have also changed. It looks like the maintainer is not active anymore so making my changes on a forked version.

ayushmanchhabra avatar May 01 '22 20:05 ayushmanchhabra

@gingerbeardman If you're looking for something production ready, this is not it. It'll probably take me 1-2 months to get everything working.

ayushmanchhabra avatar May 02 '22 00:05 ayushmanchhabra

Hey no worries! I'm not in a rush and I'm happy enough with my butchered version for now.

My current issue: on the import dialog I'm setting sprite sheet frame size w/h from values in the filename (eg. image-table-16-32.png), which works just fine, but I can't get the grid to update/display without user input (eg. tabbing to another field).

Is there anywhere we can chat about these things? This issue seems to not be the best place. @ayushmxn

gingerbeardman avatar May 02 '22 08:05 gingerbeardman

@ayushmxn i've contacted you on https://gitter.im

gingerbeardman avatar May 03 '22 08:05 gingerbeardman

@bcharbonnier Could you add @ayushmxn as a maintainer? I'm trying to package piskel as a Flatpak and this PR is basically essential for it. Without the PR I would have to package his version, which is weird

dginovker avatar Jul 24 '22 16:07 dginovker

Well this was mighty unfortunate.

dginovker avatar Oct 13 '22 08:10 dginovker

Closed the PR since didn't look like it was going to be merged so forked the repo and maintaining my own version here: https://github.com/pahuch/piskel

ayushmanchhabra avatar Oct 13 '22 17:10 ayushmanchhabra

Hello, just wondering if these fixes for the build process issues are still working or if its something I'm doing wrong or if there's another repo with a working version of it somewhere? However, if not, that is all good and thank you for your time.

NiasuK avatar Mar 22 '23 00:03 NiasuK

@NiasuK You can apply the patches in this PR and test it out - will probably have to update bunch of deps again. I used to maintain a fork but deleted it a while ago - moved on to other things.

ayushmanchhabra avatar Mar 22 '23 01:03 ayushmanchhabra