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

tweak(decrapped/native): remove files not supported #2729

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

Conversation

PsychoShock
Copy link
Contributor

Goal of this PR

Following natives that are not supported anymore, this PR will remove from the docs.

How is this PR achieving the goal

Remove natives not supported anymore.

This PR applies to the following area(s)

Natives

Successfully tested on

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

Fixes issues

Everything is already closed.

@FabianTerhorst FabianTerhorst self-requested a review August 19, 2024 16:12
@FabianTerhorst FabianTerhorst self-assigned this Aug 19, 2024
Copy link
Contributor

@FabianTerhorst FabianTerhorst left a comment

Choose a reason for hiding this comment

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

Looks good to me. These are the changes from #2666 with cleaned up commit history.

@FabianTerhorst FabianTerhorst added the ready-to-merge This PR is enqueued for merging label Aug 19, 2024
@github-actions github-actions bot added the triage Needs a preliminary assessment to determine the urgency and required action label Aug 19, 2024
@FabianTerhorst FabianTerhorst removed the triage Needs a preliminary assessment to determine the urgency and required action label Aug 19, 2024
@neptunium-cfx neptunium-cfx added triage Needs a preliminary assessment to determine the urgency and required action and removed ready-to-merge This PR is enqueued for merging labels Aug 19, 2024
@TheIndra55
Copy link
Contributor

Isn't removing SET_SNAKEOIL_FOR_ENTRY a breaking change since it's currently implemented in adhesive?

Need to be keep in docs
@github-actions github-actions bot added invalid Requires changes before it's considered valid and can be (re)triaged and removed triage Needs a preliminary assessment to determine the urgency and required action labels Aug 19, 2024
@neptunium-cfx
Copy link
Contributor

neptunium-cfx commented Aug 19, 2024

Isn't removing SET_SNAKEOIL_FOR_ENTRY a breaking change since it's currently implemented in adhesive?

It might be yeah, removed the ready-to-merge label for now, so that others can re-evaluate. (Given i have no idea how the code gen system works)

The SET_SNAKEOIL_FOR_ENTRY native is currently implemented in fivem-private, it shouldn't be used anymore, but i'd expect removing it to break some resources on a rare occasion. Ideally this native would be deprecated in a more 'friendly' way in the future.

@PsychoShock
Copy link
Contributor Author

I'll speak in intern and comeback to a solution here when I'll have one for that specific native. Others should be fine.

@PsychoShock
Copy link
Contributor Author

After talking, I think this is the "best" current way to implement the files removed. For hiding the rest used by adhesive, I will work on a different PR.

@prikolium-cfx
Copy link
Contributor

Thanks for your contribution, but we don't remove declarations to not break compatibility with existing scripts.
You can leave only your warning message changes and then we can accept it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Requires changes before it's considered valid and can be (re)triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants