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

Make Vectors immutable #188

Closed
AstraLuma opened this issue Nov 21, 2018 · 6 comments · Fixed by #204
Closed

Make Vectors immutable #188

AstraLuma opened this issue Nov 21, 2018 · 6 comments · Fixed by #204

Comments

@AstraLuma
Copy link
Member

Should we just make vectors immutable things? That seems like best practice?

@pathunstrom
Copy link
Collaborator

An issue for ppb_vector, but yeah. I think that's a thing we should probably do.

@AstraLuma
Copy link
Member Author

An issue for both. It's pretty easy on the ppb-vector side. Problem is making sure all the use in ppb conforms to it.

@ReblochonMasque
Copy link

What is the rationale for making vectors immutable? Why is it important?
Maybe it is a good idea, but in view of the many problems it creates (opened issues), maybe it is okay not to?

@nbraud
Copy link
Contributor

nbraud commented Apr 17, 2019

@ReblochonMasque ppb-vector was changed to make the vectors it provides immutable, and that change is first available in version 1.0a1.
There are a couple of places where pursuedpybear was mutating vectors in-place, and those needed to be updated (which is done in #204) for compatibility with newer versions of ppb-vector.

I'm not sure what are the issues you are referring to (esp. in ppb itself), but the rationale is pretty simple: it's both convenient and efficient to reuse vectors (say, if you are teleporting a player to a location, player.position = teleporter.position is the natural thing to do), but if those vectors are later-on mutated in-place, it will have unexpected effects (like someone moving the teleporter could also move players who teleported).

Since ppb is an engine that focuses on ergonomics and learnability, it makes sense to simply avoid surprising behaviours such as this one. Especially when the only cases of mutation we could find where internal to the engine itself.

@pathunstrom pathunstrom changed the title Make Vectors immutable? Make Vectors immutable Apr 17, 2019
@ReblochonMasque
Copy link

okay, thank you!

@pathunstrom
Copy link
Collaborator

I already liked @nbraud's response, but to confirm: as the person who made the decision to make them mutable in the first place, the least surprise principle absolutely applies and is why I've been behind this change since shortly after it was suggested.

bors bot added a commit that referenced this issue May 5, 2019
204: Immutable vectors r=astronouth7303 a=astronouth7303

Refactor the sides APIs to not mutate Vectors anymore.

Closes #188 

(Note: This is WIP because it depends on Vector 1.0 APIs. It is otherwise ready for review.)

Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
@bors bors bot closed this as completed in #204 May 5, 2019
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 a pull request may close this issue.

4 participants