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

Refactor the game map to permit exposing it to Lua #4580

Open
wants to merge 3 commits into
base: master
from

Conversation

@CelticMinstrel
Copy link
Member

CelticMinstrel commented Nov 18, 2019

Basically what this does is add wesnoth.get_map() which returns the same thing as the mapgen kernel's wesnoth.create_map() only referring to the actual map currently in use by the game.

I haven't had a chance to test it yet, but I'm opening it up for code review now.

@CelticMinstrel CelticMinstrel requested a review from gfgtdf Nov 18, 2019
@gfgtdf

This comment has been minimized.

Copy link
Contributor

gfgtdf commented Nov 18, 2019

i am not sure whether they way you cast with lua_map_ref etc is guraneted to work in the c++ standard. For exampel in the following code:

class A { int a; };
class B { int b; };
class C : public A, public B {};

void main()
{
 C* ptr0 = new C
  void* ptr1 = static_cast<void*>(ptr0)
  B* ptr2 = static_cast<B*>(ptr1)
  C* ptr3 = static_cast<C*>(ptr2)
}

ptr3 will be unequal to ptr0, now this is clearly because C also inhertis from A, but i am no sure that the C++ standard guarantees this even when there is no other A that is inherited from.

@CelticMinstrel

This comment has been minimized.

Copy link
Member Author

CelticMinstrel commented Nov 19, 2019

I believe the C++ standard guarantees that, in the absence of virtual or multiple inheritance, a dynamic pointer cast will not change the value of the pointer.

So I'm pretty sure the way I'm using it is safe.

This adds map[{x,y}] and map[x][y] as valid ways to access the individual tiles of the map, rather than using map.set_terrain and map.get_terrain. This renders get_terrain redundant but not set_terrain (as the latter offers a third parameter).

Special locations as viewed through the map userdata are now iterable, and the length function also works.
@AI0867

This comment has been minimized.

Copy link
Member

AI0867 commented Nov 21, 2019

I believe the C++ standard guarantees that, in the absence of virtual or multiple inheritance, a dynamic pointer cast will not change the value of the pointer.

So I'm pretty sure the way I'm using it is safe.

I have attempted to check this, and still have absolutely no idea.

@CelticMinstrel CelticMinstrel marked this pull request as ready for review Nov 23, 2019
@gfgtdf

This comment has been minimized.

Copy link
Contributor

gfgtdf commented Nov 23, 2019

i think the same change that you did for map:set_terrain must also be done for map:terrain_mask, that is: calling fix_villages() as dine here if its called on the main map.

https://github.com/wesnoth/wesnoth/blob/153680400da0e1f144b90cfa0fde8905fa0d674b/src/scripting/game_lua_kernel.cpp

@gfgtdf

This comment has been minimized.

Copy link
Contributor

gfgtdf commented Nov 23, 2019

also, please test that the 'maritime test' mapgenerator still works, you can access it via the editor -> generate map

@CelticMinstrel

This comment has been minimized.

Copy link
Member Author

CelticMinstrel commented Nov 23, 2019

Thank you, I was looking for a way to test the map generator side of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.