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

Back-ported some PAWN 4 natives and features. #287

Merged
merged 2 commits into from
Oct 30, 2018
Merged

Conversation

Y-Less
Copy link
Member

@Y-Less Y-Less commented Feb 25, 2018

What this PR does / why we need it:

Backport natives and features from PAWN 4.

Which issue(s) this PR fixes:

Fixes #

What kind of pull this is:

  • A Bug Fix
  • A New Feature
  • Some repository meta (documentation, etc)
  • Other

Additional Documentation:

This takes the default natives from PAWN 4 and puts them in PAWN 3.

@samphunter
Copy link

Wasn't there a problem with the PAWN 4 License? Or did you reimplement the feature?

@Y-Less
Copy link
Member Author

Y-Less commented Feb 26, 2018

I didn't actually check that, since I did assume they were the same license. However, I'll check the compatibility as they aren't the same.

@Dayvison
Copy link

I like this, but i don't think zeex will merge
#222 (comment)

@Y-Less
Copy link
Member Author

Y-Less commented Feb 26, 2018

@samphunter I don't think there's a problem. I just copied several files wholesale, and they can be used unmodified in other projects. This means that the VM will have different files with different licenses, but this is allowed and compatible. The only thing is I need to modify amx.h a little bit more, that was the only one where parts were combined so I need to just use a modified version of one or the other, not a hybrid.

@Dayvison I think it would be nice to extend the scope, and don't see any harm in doing so, but that's not for me to decide. @Zeex @Southclaws @YashasSamaga @Daniel-Cortez ?

@samphunter
Copy link

samphunter commented Feb 26, 2018

@Y-Less there is already a repo that improves the runtime: https://github.com/Daniel-Cortez/pawn-3.2-plus

EDIT:

Backporting from 4.0 doesn't seem to be possible due to a different license (Apache 2.0).

In the Readme of the pawn-3.2-plus repo, it is stated that back-porting is not possible due to the license difference. I knew I read that somewhere. Otherwise, I would have also assumed it would use the same license.

That being said, there are apparently already some features back-ported according to the what's new section.

PS:
The license of this version seems to be rather permissive. I think it should be possible to just use Apache License for the entire project including binaries and keep the original licenses headers in source files. However, I am not sure if merging files from both is possible, as there would have to be two separate copyright notices and both licenses would have to apply.

PPS: I think we are actually violating the license already by not adding notice about the files being modified, which is required by point 2 in the license.

@Y-Less
Copy link
Member Author

Y-Less commented Feb 26, 2018

That project makes some backwards-incompatible changes, and doesn't have all the natives that this PR does. I'd much rather keep these changes centralised if possible.

And that statement from the 3.2 repo about licenses is just wrong, as long as the files aren't merged.

@samphunter
Copy link

And that statement from the 3.2 repo about licenses is just wrong, as long as the files aren't merged.

Yes, I do believe so as well. I wrote so in the PS. Though it may be worth considering, whether the Compiler Team could contact CompuPhase about getting the 3.2 code under the Apache License. I don't think they would particularly mind and having a more standard License may be beneficial.

@Daniel-Cortez
Copy link
Contributor

Daniel-Cortez commented Feb 26, 2018

I think it's only logical to extend the project scope to the runtime improvement too. I mean, the group is already called 'pawn-lang', which implies that its activity should be focused on all aspects of the Pawn language, not only the compiler part.

Regarding this PR, the current license (zlib) is fairly permissive and simple, and I don't think mixing in new code under a much more complicated and restrictive license (which is Apache License 2.0) is a good idea - it would effectively put the whole repo under that new license (with some code still under the zlib license) since it's more restrictive than the current one and the end users would have to deal with its terms in the first place.

Also, @Y-Less, why did you need to backport the amx_Address macro? It's only used for physical addresses stored into the stack by PUSHR* opcodes, which aren't implemented in Pawn 3.2's instruction set, so some of the natives you updated won't even work.

In the Readme of the pawn-3.2-plus repo, it is stated that back-porting is not possible due to the license difference.

I probably misphrased it back when I was writing that readme (I'm not a native English speaker). Technically mixing in code under the Apache License 2.0 should be possible if it's in separate files (but IMHO it's still not a good thing to do though).

That project makes some backwards-incompatible changes

What changes do you mean exactly?

and doesn't have all the natives that this PR does

The only functions from 4.0 my fork is missing are fcopy, fcreatedir and a few other functions that operate on *.ini files, all of them are from amxfile.c. There's also floatint in amxfloat.c, but it's not even referenced in float.inc. These functions shouldn't be too hard to reimplement without mixing in any code from Pawn 4.0. I might actually even add some of them tomorrow.

@Zeex
Copy link
Contributor

Zeex commented Feb 26, 2018

  1. I'm a little confused about the purpose of this - are you embedding this runtime into somewhere else (i.e. not SA-MP)?

  2. Do these natives work? I remember there was a breaking change in 4.0 in how AMX addresses are translated to cell * pointers (i.e. see amx_Address)

@Y-Less
Copy link
Member Author

Y-Less commented Feb 26, 2018

I'm a little confused about the purpose of this - are you embedding this runtime into somewhere else (i.e. not SA-MP)?

Short answer, yes.

Do these natives work? I remember there was a breaking change in 4.0 in how AMX addresses are translated to cell * pointers (i.e. see amx_Address)

Also, @Y-Less, why did you need to backport the amx_Address macro? It's only used for physical addresses stored into the stack by PUSHR* opcodes, which aren't implemented in Pawn 3.2's instruction set, so some of the natives you updated won't even work.

To answer you both at once, I included amx_Address because it was used by the new versions of some of the natives, but I edited it to work with DAT-relative offsets. Those natives were just copy-pasted wholesale.

What changes do you mean exactly?

Mostly the ones that restrict LREF/SREF, which would break YSI. I know you can disable the new VM, but then there's not much point using that fork at all.

@samphunter
Copy link

samphunter commented Feb 26, 2018

Well, as long as the compiler works on both versions of the abstract machine and the machine is backwards compatible, I guess there is no harm pulling the changes. So I would say yes to this PR.

As for me personally, I was playing around with implementing the plus runtime for samp, as I think the plus version is a much better fit. I am not sure for what the LREF/SREF hacking is needed, but I think it would be better to sacrifice it in order to properly sandbox scripts. Most of the YSI functionality can be provided by the native code, and can be implemented better in something like C++.

But there is no reason not to give people the option to chose. Having both (or all three with the original) versions available would be better.

@Y-Less
Copy link
Member Author

Y-Less commented Feb 27, 2018

Technically you could implement the YSI features in the compiler. That is something I've thought about a few times, but just never got around to it.

@samphunter
Copy link

Technically you could implement the YSI features in the compiler. That is something I've thought about a few times, but just never got around to it.

Which features do you think would be appropriate to implement into the compiler? I can only think of inlines and hooks.

IMHO inlines are too big of a change to implement into this compiler, but hooks could be useful.

@Y-Less
Copy link
Member Author

Y-Less commented Feb 27, 2018

inlines, hooks, and ___. They are the ones that use the most code generation to work.

@Y-Less
Copy link
Member Author

Y-Less commented Feb 27, 2018

It would be a compiler fork though, or at least hidden behind a flag.

@stale
Copy link

stale bot commented Oct 26, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale label Oct 26, 2018
@Y-Less Y-Less requested a review from a team as a code owner October 29, 2018 01:23
@stale stale bot removed state: stale labels Oct 29, 2018
@Y-Less Y-Less merged commit 0f52113 into master Oct 30, 2018
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.

5 participants