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

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

Closed
wants to merge 39 commits into from
Closed

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

wants to merge 39 commits into from

Conversation

ayushmanchhabra
Copy link

@ayushmanchhabra ayushmanchhabra commented Feb 11, 2022

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 ayushmanchhabra mentioned this pull request Feb 11, 2022
5 tasks
@ayushmanchhabra ayushmanchhabra changed the title Fix build related issues Fix #1047 Feb 11, 2022
@ayushmanchhabra ayushmanchhabra marked this pull request as ready for review February 11, 2022 19:26
@ayushmanchhabra ayushmanchhabra changed the title Fix #1047 Fix build process Feb 11, 2022
@gingerbeardman
Copy link

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/"]
},

@ayushmanchhabra
Copy link
Author

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.

@gingerbeardman
Copy link

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?

@ayushmanchhabra
Copy link
Author

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
Copy link
Author

@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.

@gingerbeardman
Copy link

gingerbeardman commented May 2, 2022

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
Copy link

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

@ayushmanchhabra ayushmanchhabra changed the title Fix build process chore(deps): update deps May 7, 2022
@ayushmanchhabra ayushmanchhabra changed the title chore(deps): update deps fix(build): update NW.js desktop config Jun 17, 2022
@ayushmanchhabra ayushmanchhabra changed the title fix(build): update NW.js desktop config fix(build): update deps, karma and nw config Jun 29, 2022
@dginovker
Copy link

@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

ayushmanchhabra and others added 13 commits July 24, 2022 13:04
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.4.6 to 3.14.1.
- [Release notes](https://github.com/nodeca/js-yaml/releases)
- [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md)
- [Commits](nodeca/js-yaml@3.4.6...3.14.1)

---
updated-dependencies:
- dependency-name: js-yaml
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [underscore](https://github.com/jashkenas/underscore) from 1.6.0 to 1.13.4.
- [Release notes](https://github.com/jashkenas/underscore/releases)
- [Commits](jashkenas/underscore@1.6.0...1.13.4)

---
updated-dependencies:
- dependency-name: underscore
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [lodash](https://github.com/lodash/lodash) from 3.10.1 to 4.17.21.
- [Release notes](https://github.com/lodash/lodash/releases)
- [Commits](lodash/lodash@3.10.1...4.17.21)

---
updated-dependencies:
- dependency-name: lodash
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@ayushmanchhabra ayushmanchhabra closed this by deleting the head repository Sep 29, 2022
@dginovker
Copy link

Well this was mighty unfortunate.

@ayushmanchhabra
Copy link
Author

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

@NiasuK
Copy link

NiasuK commented Mar 22, 2023

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.

@ayushmanchhabra
Copy link
Author

@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.

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.

Build process broken
4 participants