Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore building cars for bounding box calculation #22

Closed
fortysixandtwo opened this issue Mar 16, 2021 · 10 comments · Fixed by #23
Closed

Ignore building cars for bounding box calculation #22

fortysixandtwo opened this issue Mar 16, 2021 · 10 comments · Fixed by #23

Comments

@fortysixandtwo
Copy link
Contributor

Hey,

first of all thanks for your mod. It looks really nice and I'm amazed by the possibilities (different kinds of trackers for the camera).

The problem I encountered is that the "viewport" of the camera shows a lot of non base stuff as can be seen
00000861-basecam
After looking through the code and talking with one of the players I believe the issue is due to the player building/placing his/her car far from the base while on an exploration run.

If I understand the code correctly (I don't know Lua at all) the bounding box for the
base is recalculated in https://github.com/veger/TLBE/blob/master/scripts/main.lua#L77 with the updating
done in https://github.com/veger/TLBE/blob/master/scripts/main.lua#L99

The function Main.entity_built is a callback function registered in https://github.com/veger/TLBE/blob/master/control.lua#L58
I suggest (I can probably come up with a Pull Request) to ignore any kind of player drivable vehicle in this callback.

Do you think it would be enough to "fix" the base size if I destroyed the cars/drove them closer to base and hit "recalculate base size"?

Or would I need to use the area tracker for my camera?

Thanks in advance!

@veger
Copy link
Owner

veger commented Mar 17, 2021

Thanks you for your thorough report. I think you could be on the right track.
TLBE does not check what got build, I never thought of this use-case... 😉 So 'building' a car or some other vehicle outside of the current 'base area' will definitely influence the tracked area.

'Recalculate base size' should solve this (if the car is 'inside' the base area) as it checks all entities again in order determine the total base size.

For a permanent solution, it seems to make sense if we could indeed check what type of entity got build and if it should count as part of the base. So ignoring vehicles seems nice, but I also wonder if combat robotic influence this... (and maybe other entities as well??)

fortysixandtwo added a commit to fortysixandtwo/TLBE that referenced this issue Mar 17, 2021
This patch uses LuaPlayerBuiltEntityEventFilters to make sure
that the `TLBE.Main.entity_built ()` callback is not called for
vehicles.

Fixes veger#22
fortysixandtwo added a commit to fortysixandtwo/TLBE that referenced this issue Mar 17, 2021
This patch uses LuaPlayerBuiltEntityEventFilters to make sure
that the `TLBE.Main.entity_built ()` callback is not called for
vehicles.

Additionaly during base size recalculation we skip entities of
type `car` (currently cars and tanks).

Fixes veger#22
@fortysixandtwo
Copy link
Contributor Author

See #23 :)

Haven't tested it yet though!

@veger
Copy link
Owner

veger commented Mar 18, 2021

I'll run a few quick tests soon 😉

@fortysixandtwo
Copy link
Contributor Author

I wanted to test drive this as well, but haven't yet found time to start up Factorio.

Also please note, that this is the first time I ever wrote Lua (pretty nice that Factorio ships the docs in doc-html/).

And what is up with 1-base arrays in Lua? :D

@fortysixandtwo
Copy link
Contributor Author

I did some testing and while it's not exactly how i imagined it - still a few things on the screenshot I wouldn't have expected - it seems a bit better than before.

F.e.
00000872-basecam

Adding a car in the north: (viewport does not change)
00000873-basecam

Adding a turret in the north (viewport does change - it doesn't show the turret, but maybe thats because of aspect ratio of the screenshot or something)

Uploading 2 screenshots because I have smooth camera

00000874-basecam

00000875-basecam

@fortysixandtwo
Copy link
Contributor Author

So it appears to be working, but please run some tests of your own ;)

@veger
Copy link
Owner

veger commented Mar 19, 2021

Thank you for your tests and screenshots!

TBH I would expect the first 2 screenshots zoomed in a little more, as the South and West seem empty the zoom level could be higher... 🤔

I'll try and test this weekend when I have some time.

@fortysixandtwo
Copy link
Contributor Author

fortysixandtwo commented Mar 20, 2021

I had a session today and it seems good enough (even if it the screenshots are not ideal yet).
Not sure why the area in the west is included (or if this is a consequence of the 16:9 aspect ratio).

In case you're interested you may have a look at https://fortysixandtwo.eu/upload/basecam.mp4 (might not work in browser, but rightclick -> save and then playing with vlc do the trick - at least that's what my windows using friends had to do).

@veger veger closed this as completed in #23 Mar 21, 2021
@veger
Copy link
Owner

veger commented Mar 21, 2021

When looking at the movie it looks like the height fits (almost? hard to see since it looks like you adjusted/recalculated the area?) perfectly, so I am pretty sure it is the 16:9 aspect ratio indeed.

@veger
Copy link
Owner

veger commented Mar 21, 2021

Fixed in v1.4.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants