Wiwiweb / FactorioMilestones

Factorio Mod for tracking your milestones
GNU General Public License v3.0
9 stars 15 forks source link

Crash on add to empty list #17

Open digidoor opened 8 months ago

digidoor commented 8 months ago
The mod Milestones (1.3.25) caused a non-recoverable error.
Please report this error to the mod author.

Error while running event Milestones::on_gui_click (ID 1)
__Milestones__/scripts/gui_settings_page.lua:284: attempt to index field 'milestones_settings_checkbox' (a nil value)
stack traceback:
    __Milestones__/scripts/gui_settings_page.lua:284: in function 'update_checkbox_states'
    __Milestones__/scripts/gui_settings_page.lua:335: in function 'select_setting'
    __Milestones__/scripts/gui.lua:300: in function <__Milestones__/scripts/gui.lua:260>

Reproduction:

  1. Open Milestone interface with Ctrl+Alt+M
  2. Click settings
  3. With Vanilla selected (by default) click the Export icon button.
  4. Copy with Ctrl+C
  5. Click Close
  6. Use dropdown to switch from Vanilla to Empty
  7. Click Import icon button
  8. Ctrl+V to paste buffer of previously copied stuff into the new import window
  9. Click the Import text button at the bottom of this new window
  10. Click Add Research button (or any of the others like Add Kill or Add Group)
  11. Click empty milestone placeholder box that results
  12. Click first icon for the Automation milestone (or any other potential milestone; doesn't matter which one)
  13. Click checkbox on the right of row (that has tooltip "Select to delete this milestone yadda yadda yadda") or the arrow to change order.

Bam, crashes. Steps 11 and 12 can be skipped and the same crash will occur.

The problem is at step 10: after importing, the Add Group/Item/Research/etc. buttons are clickable even though no checkbox is selected and the Milestone list is not empty. When any of the Add buttons are clicked, they make a ghost milestone that isn't attached to anything, and then clicking its checkbox (or the little down arrow to change its position in the list) leads to a crash. Presumably this is possible to allow something to be added to an empty Milestones list.

Another way to trigger it (same steps 1-5):

  1. Open Milestone interface with Ctrl+Alt+M
  2. Click settings
  3. With Vanilla selected (by default) click the Export icon button.
  4. Copy with Ctrl+C
  5. Click Click Import icon button. (Do not click the Import text button in the new resulting window.)
  6. Switch Preset from Vanilla to Empty using the dropdown clicker.
  7. NOW Paste valid milestones JSON into the import window.
  8. Click the Import text button.
  9. The window will automatically switch back to Vanilla. Repeat steps 10 through 13 (or just 10 and 13 since 11 and 12 aren't necessary to trigger the crash) since the Add buttons are live.

The final and simplest way to trigger the crash is to import something to the Empty list. The Add buttons will still be active and then steps 10 & 13 can be repeated for a crash. The trigger is simply to do a successful import when the Empty list is selected and then try to Add something. The resulting item cannot be selected or moved without a crash.

I ran into the bug because I was hoping to make my own list to use on any playthrough, but I don't think mods can write or read to/from external files. If I make my own list and rezip the mod to enable it, would I be able to change the list later on an existing save without causing a crash, do you think?

Wiwiweb commented 8 months ago

Nice catch! And thanks for the detailed repro instructions. I think the crux of the bug is that starting from an empty preset and then using the "import" feature still allows you to add a milestone without selecting a checkbox, as if the list was still empty. That puts the settings screen in a corrupted state.