veger / TLBE

TLBE - Time Lapse Base Edition
MIT License
9 stars 6 forks source link

The code to provide a city block tracker #52

Closed drdozer closed 11 months ago

drdozer commented 1 year ago

See #45

drdozer commented 1 year ago

I'm getting this error:

Error while running event TLBE::on_gui_selection_state_changed (ID 60)
__TLBE__/scripts/gui.lua:1560: attempt to index field 'tlbe-tracker-cityblock-size-x' (a nil value)
stack traceback:
    __TLBE__/scripts/gui.lua:1560: in function 'updateTrackerConfig'
    __TLBE__/scripts/gui.lua:1507: in function 'createTrackerConfigAndInfo'
    __TLBE__/scripts/gui.lua:459: in function <__TLBE__/scripts/gui.lua:408>

I'm not quite sure why it is happening. I copied and adapted the code for the area tracker, so I am not sure why it can find the area tracker fields but not the city block fields.

veger commented 1 year ago

You put the city block GUI inside its own 'flow' (here) and on line 1560 you are reading from the 'main' tracker flow (trackerInfo). So your field is not found (nil) and you call text on it. Causing the error.

Instead you first need to fetch that 'flow' and then index your fields, like it is done here

veger commented 1 year ago

BTW You master branch is outdated, that is why you see all your (old) commits... You need to (re)sync it with the main/this repo. With forks (but in general as well) you should create a 'feature branch' and work in there. So the main/master branch stays clean and is easy to sync.

drdozer commented 1 year ago

I'll start again tomorrow - it's getting late. Also, yes, I will use feature branches in future.

drdozer commented 1 year ago

Ready for review I think.

drdozer commented 1 year ago

Thanks for the feedback. I'll work through them when the house is empty of rugrats.

drdozer commented 1 year ago
  • Changing the decimal of the scale is very awkward: delete the 5 and a 0 pops up, so I cannot fill in 1.25 for example.

I've fixed the update function to round to two sf, just like the display function does. This should be fixed by that, but GUIs are eldrich horrors.

drdozer commented 1 year ago
  • Changing the decimal of the scale is very awkward

Yes -- it is a weird interaction of the logic for setting the value into the field and setting the value into the data. I will have a cup of tea and maybe ask in discord, as I'm a little stumped as to how to fix it.

drdozer commented 1 year ago

GUIs are eldrich horrors

I'm a bit stuck with this issue of the scale decimal value. I think I need to validate/set the value in some event that is not onTextChanged, as it is "fixing" the number on keypress, which means it gets reformatted on each keypress.

drdozer commented 1 year ago

Possible solution pushed.

veger commented 1 year ago

GUIs are eldrich horrors

agree yes... unfortunately they are a necessary evil sometimes.... :imp:

I'll fetch your newest changes, and give it another try

drdozer commented 1 year ago

I think allowing block size = 0 is not good.

Agreed. Now, let's see if I can address this without more GUI eldrich horrors being invoked. I've pushed what I hope is a fix.

drdozer commented 1 year ago

Also, the game hung on me once, had to force quit

It has hung on me a couple of times also. I have not been able to work out where or why it is hanging. My first thought was some interaction with the vs code plugin. I haven't yet had a hangup when running factorio free-range.

drdozer commented 1 year ago

Currently without Internet due to a broken 4g mast in our area...

veger commented 11 months ago

I would like to include this new tracker in the coming release, which I plan do to soon-ish so others can enjoy all the new additions to the mod.

Can you indicate whether you can spend some time on this PR, or shall I release without the new city block tracker?

drdozer commented 11 months ago

Sorry - life happened. What's outstanding for this to be released?

veger commented 11 months ago

Sorry - life happened

No worries. Same here, so I didn't do a release yet

I think the following issues are still open:

I think they are mostly small clean up changes

drdozer commented 11 months ago

I think they are mostly small clean up changes

I think they are all now resolved.

veger commented 11 months ago

nice thanks a lot for this feature!

I'll try and find some this later to make the release