carenalgas / popochiu

Godot plugin to make point-and-click adventure games, inspired by tools like Adventure Game Studio and PowerQuest.
https://carenalgas.github.io/popochiu/
MIT License
205 stars 19 forks source link

refs #343: Fix walk and look points storage and retrieval #344

Open mapedorr opened 1 week ago

mapedorr commented 1 week ago

Fixes #343 : The data should be stored using built-in types (int, float, String, and so on) in order to avoid hacks in Web games. A validation was added to assign the values for the walk_to_point and look_at_point based on the available data in the character's Dictionary.

stickgrinder commented 4 days ago

I would move this into a couple of functions, named something like pack_vector_state and unpack_vector_state (or anything that makes sense).

I agree with @balloonpopper about hardening the data with prior validation.

mapedorr commented 1 day ago

@balloonpopper @stickgrinder I made an update using RegEx to validate the format of the stored String.

Regarding the creation of a function to pack those vectors (PopochiuCharacter.walk_to_point and PopochiuCharacter.look_at_point), I’m thinking about drafting a proposal to add a feature that automatically stores any Vector2 property in objects saved as a JSON dictionary, with x and y keys and values.

Let me know what you think, and if it’s okay to merge this PR as it is (if approved), leaving the pack_vector_state function for the proposal I’m referring to.

balloonpopper commented 6 hours ago

This looks complicated to me. Can we get away with just checking if it's the expected format as only Popochiu should be setting it and therefore the state should be known. If you want to make it support all the use cases though (presumably you might be planning to use this in other places?) then it looks good.

I might be reading it wrongly (I'm not familiar enough with the codebase), but isn't the default case a dictionary? If so, I'd suggest putting the dictionary check before the regex.