search

sandalle / minecraft_bigreactor_control

Minecraft BigReactor Computercraft Control Program
MIT License
76 stars 41 forks source link

Changes to ComputerCraft DeviceID cause an out-of-bounds index reference when loading a now non-existent config #53

Closed dkowis closed 9 years ago

dkowis commented 9 years ago

Don't know why it doesn't work, yet: failure

Although there should only be one turbine, not two...

dkowis commented 9 years ago

I had to delete a turbine configuration, somehow it decided there was more than one :(

sandalle commented 9 years ago

Do you have two modems to a single Turbine? Were there two Turbines at some point? Was the program restarted after a Turbine was added/removed? The program auto-discovers connected devices every loop and should only crash if the devices (monitors, turbines, reactors, etc.) are added/removed while it's running.

dkowis commented 9 years ago

Only one turbine ever. Never had two modems on it. Program was restarted after turbine added.

Nothing was changed, I leave it running for a long time and come back and it's le crashed :(

sandalle commented 9 years ago

From what I can see in the code, that's trying to refer to a Turbine that your computer reported as being attached and was found by the program, and then when trying to save that second Turbine's information, we get an out-of-bounds error (nil).

sandalle commented 9 years ago

Was your Turbine perhaps attached to a different modem earlier (e.g. you replaced the modem connecting it) and so got a different ID from ComputerCraft and shows as a different Turbine (even if you only have one)? The function findTurbines() loads the config files based on name, see https://github.com/sandalle/minecraft_bigreactor_control/blob/master/lolmer_bigreactor_monitor_prog.lua#L917.

dkowis commented 9 years ago

that might have happened....

sandalle commented 9 years ago

In config.load(path) we check if the file is opened properly for read and return "nil" if not, and then in findTurbines() we try to load that file, but if it's nil we then create the file with default values so it's valid on the next run. So unless turbineIndex somehow changed while the program is running, we should not be indexing nil.

sandalle commented 9 years ago

Does this still happen after removing the file? Can you try replacing the modem to get a new Turbine ID from ComputerCraft and see if you can reproduce the error? Line 916 from release 0.3.16 is checking for LastSpeed, which has a default value set on Line 889 from release 0.3.16 and so should not be nil as we only overwrite the default value if the value loaded from the config (if the config is not nil) is also not nil.

sandalle commented 9 years ago

@dkowis Could you test with my test branch which has the code commit from above and debugMode=true and then try to replicate? :) You can just modify your startup script to download the pastebin above instead of the stable code.

sandalle commented 9 years ago

Please re-open if you can duplicate the bug and provide the debug log so that we can track it down, but I cannot replicate the issue here.