search

clintbellanger / flare

Free Libre Action Roleplaying Engine
http://clintbellanger.net/rpg/
GNU General Public License v3.0
166 stars 41 forks source link

Check basic input data for consistency #907

Closed igorko closed 11 years ago

igorko commented 11 years ago

For issue #754. Guys, please check the code carefully. It was tested only for crashes. Maybe some data still needs check at input, but it will need more game testing from players/moders side.

stefanbeller commented 11 years ago

In the given commit there are checks like 

id > MAX_INT -1

This check is only true for id == MAX_INT, because there are no further numbers above MAX_INT. (It wraps around to the negative side) So these checks for being too large is not effective, but only bloats the program.

igorko commented 11 years ago

YES. Afaik if you will read from file larger number, than int, it will be trunkated and assigned value will be MAX_INT. So that checks if number in data file is equal or larger.

I had an idea to convert input data ints to shorts, because I think we will never use such large numbers. In case of short such check is usefull, but it should be replaced to SHORT_MAX.

igorko commented 11 years ago

Heh. Value over MAX_INT is dropped to 0, not to MAX_INT. it's bad, we need to make it work somehow...

stefanbeller commented 11 years ago

The toInt function lets you set a default, which could be either MAX_INT or 0. We're also ignoring 0 right?

igorko commented 11 years ago

No, some things (like powers) begin from 0, some (like items) begin from 1. So we need to handle only top fringe.

stefanbeller commented 11 years ago

maybe the default of the toInt could then be set to -1, which is always indicating failure?

igorko commented 11 years ago

We could do that, but I wonder if this can create side effects in other places of code.(0 is unsigned, -1 is signed)

clintbellanger commented 11 years ago

Because it's necessary to specify "no power" and "no item" in various places (e.g. equipment, action bar) perhaps we can change powers to use 0 as the "no power" value. It'll help some with consistency. I think power #0 is "Shoot", which we can move to another ID (several creatures will be affected).

stefanbeller commented 11 years ago

when reading a -1 into an unsigned variable, it gets converted to MAX_INT according to http://en.wikipedia.org/wiki/Two%27s_complement so then the checks introduced for checking on 0 and MAX_INT would make sense again :)

igorko commented 11 years ago

toInt() returns signed int, so this is not our case. I'll commit changes soon to remove unneeded checks.