search

Mwexim / skript-parser

A standalone and improved implementation of the Skript language for use outside of Minecraft servers.
MIT License
49 stars 15 forks source link

Functions & Structures 🎉 #156

Open hapily04 opened 10 months ago

WeeskyBDW commented 9 months ago

Hello First at all, welcome and thanks to contribute to the project. Dont stress yourself with git stuff, if necessary a skript-parser maintainer would (help you to) fix theses. Also be carefull when changing forked project licence as if you didn't ask to projet author (and contributors) you can, in open-source project (mostly with biggest ones) have legal issues (intellectual property/copyright)

WIll see if i can be usefull about that PR but prob not

Mwexim commented 9 months ago

I see a lot of interesting changes but also some things that do not follow the project's guidelines. Functions have never really been discussed.

Therefore, I will leave this pull request open for discussion and once I find some more time to work on the project again, I will happily talk about this pull request with you.

The concerns I currently have:

hapily04 commented 9 months ago

I see a lot of interesting changes but also some things that do not follow the project's guidelines. Functions have never really been discussed.

Therefore, I will leave this pull request open for discussion and once I find some more time to work on the project again, I will happily talk about this pull request with you.

The concerns I currently have:

  • Code style, the java version upgrade

To be quite frank the java version upgrade appears to be for the (x instanceof y variableName) which I use in my own code typically. I originally made all of these changes/additions/fixes for my own fork, but thought it'd be useful to share.

  • The function pattern is the same as in vanilla Skript, but nobody actually discussed if an overhaul would be preferred

Like I said before, a lot of these things are for my own fork which aims to be pretty similar to Skript so that it's easier for anyone wanting to switch. If there was a better proposal for function syntax I wouldn't be immediately opposed to still contribute and edit my contributions, but will keep my implementation for my own fork.

  • The code changes some minor things that have nothing to do with the pull request

I agree and this has a lot do with me not committing correctly in preparation for a pull request. I'm unsure how to fix any of this without going through the burden of creating a new fork and adding my changes one by one.

If you could help guide me in how to fix both of my pull requests (and potentially split some parts of them into different pull requests) that would be extremely helpful.

Mwexim commented 9 months ago

Feel free to keep adding pull requests for features you're adding to your own fork, it keeps the project updated and discussion going. I will always try to discuss the changes and help you through the process. The only thing I'd like to stay is the Java version.

This particular pull request though just changes too many things at once, and would need some major discussion and changes for it to be merged. It's also a pretty close copy of Skript's system, and skript-parser always tries to optimize things and see if we could handle things differently before copying certain features. Did you have this mind-set when adding functions to your fork?

I currently lack the time to guide you through this as I would need to think for myself if Skript's way is indeed the right way to do things. That said, I really welcome your contribution and future contributions. If we ever add functions to skript-parser, this pull request will certainly add as a good reference on where to start!