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

transferVBL macro function is misnamed #189

Closed
Phergus opened this issue Jun 30, 2018 · 7 comments
Closed

transferVBL macro function is misnamed #189

Phergus opened this issue Jun 30, 2018 · 7 comments
Labels
feature Adding functionality that adds value

Comments

@Phergus
Copy link
Contributor

Phergus commented Jun 30, 2018

The transferVBL() function is somewhat misnamed. If you transfer something it is moved from one place to another. The function is really a copyVBL() function.

Changing this might break existing macros but it would be useful if it did what it said.

Alternatively a second parameter that determined transfer or copy could be added. This might also break existing macros as the current second parameter, token ID, would need to be pushed to the third slot.

@JamzTheMan
Copy link
Member

Agreed. We could add add a copy of function and deprecate the current.

We could also use logic to deprecate functions :)

@Phergus
Copy link
Contributor Author

Phergus commented Jul 11, 2018

Use logic? Inconceivable!

@JamzTheMan JamzTheMan added the up for grabs Minimal context and are well-suited for new contributors label Sep 26, 2018
@JamzTheMan
Copy link
Member

Marking some issues as "Help Wanted" that could be good places to start for new developers...

@source-knights
Copy link
Contributor

Adding my opinion: I don't like copyVBL() either cause it would suggest I do a paste later. So I would more be a copyVBLTo…

What I suggest is adding an optional parameter to the transferVBL function.
transferVBL(value, id, delete)

Then if delete is true it actually deletes it, so equal to a real transfer/move.

That way we do not hit existing macro code and I guess its still doing what the idea of this Jira is.

If you guys agree and still want it I can add that.

By the way, the much bigger problem with VBL is the different scale between token and normal VBL (also just my POV ;) )

@Phergus
Copy link
Contributor Author

Phergus commented Mar 15, 2019

Normal MT macro function usage is to have the token ID as the last, optional parameter.

So it would be:

transferVBL(to/from)
transferVBL(to/from, delete)
transferVBL(to/from, delete, id)

While that might break a handful of existing macros it shouldn't be a big impact as the function is probably not used much in framework code.

@source-knights
Copy link
Contributor

source-knights commented Mar 15, 2019 via email

@rkathey rkathey added feature Adding functionality that adds value and removed up for grabs Minimal context and are well-suited for new contributors labels Mar 15, 2019
@source-knights
Copy link
Contributor

Fix in #310
Amended to use a new Boolean optional delete parameter, fully backwards compatible.

cwisniew added a commit that referenced this issue Mar 22, 2019
Fix #189 amend transferVBL function to really transfer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value
Projects
None yet
Development

No branches or pull requests

4 participants