search

SpaceManiac / SpacemanDMM

A BYOND language smartness provider, map renderer, and more.
https://marketplace.visualstudio.com/items?itemName=platymuus.dm-langclient
GNU General Public License v3.0
83 stars 83 forks source link

dmm_tools::dmm::Map::adjust_key_length() fix #379

Closed DreamySkrell closed 1 year ago

DreamySkrell commented 1 year ago

Currently, there is Map::adjust_key_length(), but it sets key length based on the amount of items in the dictionary. Which is fine if keys in order like 1-2-3-4-...-N, but it falls apart if the keys are not in order - which is valid for a dmm map.

If a map has keys like [1, 2, 3000], then, currently, Map::adjust_key_length() will set key length to 1, but the map will panic on save cause the 3000 key is bigger than max key for this key length (52).

This PR fixes adjust_key_length to work properly in that case, where it sets key length based on the biggest key in the map dictionary.

SpaceManiac commented 1 year ago

Perhaps this function should validate that the passed-in length is 1, 2, or 3.

Perhaps adjust_key_length should also be fixed!

DreamySkrell commented 1 year ago

Perhaps this function should validate that the passed-in length is 1, 2, or 3.

Perhaps adjust_key_length should also be fixed!

Now thinking about it, fixing adjust_key_length would be better and safer, where it would get the largest key in the dictionary, and set key length based on that.

Even with the validation for set_key_length as written currently, the user could still give it 1 when there's too many keys, and it would panic on save.

I will do this later today.

DreamySkrell commented 1 year ago

Updated code and PR as above.