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

Problem since luabind replace by Sol2 #3452

Closed
fijemax opened this issue Dec 15, 2016 · 13 comments
Closed

Problem since luabind replace by Sol2 #3452

fijemax opened this issue Dec 15, 2016 · 13 comments
Milestone

Comments

@fijemax
Copy link
Contributor

fijemax commented Dec 15, 2016

Since that luabind was replaced by Sol2 I can't iter on way's nodes.

My lua code :

  print("way id : ", way:id());
  for node in way:get_nodes() do
   print("  node id  : ", node:id())
  end

The error exception :

way id : 	5203906
TBB Warning: Exact exception propagation is requested by application but the linked library is built without support for it
terminate called after throwing an instance of 'tbb::captured_exception'
  what():  lua: error: profile.lua:677: attempt to call a userdata value
Abandon (core dumped)
@daniel-j-h
Copy link
Member

Thanks,

@karenzshea can you have a look? This could be related to the notes here.

@fijemax can you tell us the Lua version you are using? 5.1 by chance?

@fijemax
Copy link
Contributor Author

fijemax commented Dec 15, 2016

@daniel-j-h
Lua 5.1.5 Copyright (C) 1994-2012 Lua.org, PUC-Rio

@fijemax
Copy link
Contributor Author

fijemax commented Dec 15, 2016

I can solved the problem by updating lua ?

@daniel-j-h
Copy link
Member

Maybe; you can give Lua 5.2 a try.

But that's still an issue for us since we support both Lua 5.1 and Lua 5.2.

Lua 5.3 has some garbage collection issues which make it consume way more memory during extraction.

@karenzshea karenzshea removed their assignment Dec 15, 2016
@danpat
Copy link
Member

danpat commented Dec 17, 2016

I can reproduce this, and unfortunately simply switching to Lua 5.2 doesn't fix it. Needs some more digging, but it looks like we're not binding everything we need to with Sol2.

@oxidase
Copy link
Contributor

oxidase commented Dec 17, 2016

@fijemax in your example for node in way:get_nodes() do will not work with #3468 but for _, node in ipairs(way:get_nodes()) do should be ok. I don't see a way to make an iterator with a state in sol.

@fijemax
Copy link
Contributor Author

fijemax commented Dec 20, 2016

Hi,
I tested with for _, node in ipairs(way:get_nodes()) do
this don't work no more.

@oxidase
Copy link
Contributor

oxidase commented Dec 20, 2016

@fijemax the fix is not yet merged to master but located the branch sol-metafunctions.
It must be clarified first how to deal with iterators for user defined types in sol2.

@ThePhD
Copy link

ThePhD commented Dec 21, 2016

For Lua 5.2 and 5.3, you can override the appropriate iteration metamethods (pairs, next, etc.). We do that when we serialize userdata containers (implementation here where we store a userdata iterator and call the state with whatever begin and end iterators provided by the class, if they exist).

For Lua 5.1, if and only if you want to use ipairs or pairs, you must hand the function a table of some sort (you have to serialize whatever data to a table). This makes it read-only and also makes it hard to modify elements during iteration (if that is what you require), but that's just how it goes (unless you iterate in some other manner).

Note that using a numeric for loop and somehow associating some state data with that might also be beneficial for your purposes.

@MoKob
Copy link

MoKob commented Jan 4, 2017

@oxidase does you PR fully solve this problem? If so, could you close here. Otherwise it would be great if we could capture clearly whats left to do.

@oxidase
Copy link
Contributor

oxidase commented Jan 4, 2017

@MoKob unfortunately it is not the complete solution that would require implementation of a userdata iterator and dropping Lua 5.1 support.

@fijemax please could you verify 2640a31 ? The returned value of get_nodes now is a Lua table and can be iterated with ipairs.

@fijemax
Copy link
Contributor Author

fijemax commented Jan 4, 2017

Hi @MoKob, @oxidase

  1. I just noticed that nodes accecible in the luabind in "way_function" are actually "noderefs" and this class don't contains version argument, i can't test version here.
    But I have tested node version in the "node_function" in lua and it's all good.

  2. I tested the returned value of get_nodes and I can iter on with ipairs.

I rebase this commit on master.

@oxidase
Copy link
Contributor

oxidase commented Jan 4, 2017

@fijemax thanks for your feedback! Closed as partially implemented. Full solution would require dropping Lua 5.1 support and implementation of userdata meta functions for WayNodeList.

@oxidase oxidase closed this as completed Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants