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

Local variable type hints. Helps identify local variable return types #3864

Open
TPGamesNL opened this issue Mar 28, 2021 · 4 comments
Open
Assignees
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. PR available Issues which have a yet-to-be merged PR resolving it up for debate When the decision is yet to be debated on the issue in question

Comments

@TPGamesNL
Copy link
Member

Local variable type hints were implemented in 2.2-dev37c, see dev37c release notes and bug report. These were accidentally disabled a couple months later (I think it was accidentally, but I'm not sure tho, GitHub isn't on my side for this one).

The question is: should these be brought back?
The implementation will give a warning when loading the script, when the determined type of a simple local variable (a simple local variable is one like {_a}, {_item}, {_l::1}, and not {_l::%{_i}%} or {notLocalVariable}) doesn't match the required type for its usage. For example, this code would produce such a warning:

set {_x} to "Hello, world!"
set {_y} to location of {_x}

since {_x} is a string, which doesn't have a location.

Another thing to keep in mind is addons, since some addons may change variables in different ways, this is often done when an effect is ran asynchronously, but it has a result that can be stored in a variable, for example:

These addons would have to be updated, otherwise using them with local variables will produce the warnings.

Any feedback is appreciated (:

@TPGamesNL TPGamesNL added enhancement Feature request, an issue about something that could be improved, or a PR improving something. help wanted up for debate When the decision is yet to be debated on the issue in question labels Mar 28, 2021
@TheDGOfficial
Copy link
Contributor

Yes, they are only warnings and should be probably added back. I remember it being working before and actually helped me catch some things. With a config option to disable them of course, like most other warnings.

Not related to this, but GitHub has a new discussions feature and a new issue system. Maybe look at it and enable for this repo?

As for add-ons, they should be updated if the problem can only be fixed on their side. Otherwise, AsyncEffect or other things maybe updated to correct these, if possible.

@TPGamesNL
Copy link
Member Author

I've actually just thought of something that might make it so that addons don't need updating: updating the type hints when Variable#acceptChange is called. While this isn't exactly something that should be done on acceptChange, it would prevent issues with addons, so long as the addons use the expression changers, and not Variables.setVariable.

@TPGamesNL
Copy link
Member Author

I've actually just thought of something that might make it so that addons don't need updating: updating the type hints when Variable#acceptChange is called. While this isn't exactly something that should be done on acceptChange, it would prevent issues with addons, so long as the addons use the expression changers, and not Variables.setVariable.

A lot of addons unfortunately just expr instanceof Variable instead of checking acceptChange, so this won't help

@TheLimeGlass
Copy link
Collaborator

TheLimeGlass commented Jul 13, 2022

Waiting on pull #4566 before starting this as it modifies and utilizes the type hints class.

@TheLimeGlass TheLimeGlass changed the title [Discussion] Local variable type hints Local variable type hints. Helps identify local variable return types Jul 20, 2022
@TheLimeGlass TheLimeGlass added the PR available Issues which have a yet-to-be merged PR resolving it label Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something. PR available Issues which have a yet-to-be merged PR resolving it up for debate When the decision is yet to be debated on the issue in question
Projects
None yet
Development

No branches or pull requests

3 participants