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

lua support #394

Closed
wants to merge 21 commits into from
Closed

lua support #394

wants to merge 21 commits into from

Conversation

lenscas
Copy link

@lenscas lenscas commented Feb 21, 2022

Before lua (or any dynamically typed language for that matter) can be added there needs to be a more dynamic way to query the world. This is because dynamically typed languages can't give type parameters to world.query().

The api that I am currently going for is more like a builder, in which every function adds extra type information to the query that was made.

local query = FishFight
    .build_query() --can be queried like a world.query::<()>()
    :with_player() --world.query::<&mut Player>()
    :with_player_event_queue() --world.query::<(&mut Player,&mut PlayerEventQueue)>()
    :query()

for entity, player,events in query:iter() do
    print(entity) --do actual work here
end

but, this is not set in stone and will change based on feedback and whatever else I discover.

However, it is a bit weird to turn the current way of querying the world into a Builder, as only the types are important. This also means that the current code for the builder is not exactly nice and kind of weird.

So, before I continue with this (and throw macros at the problem) I would like some feedback on it to see if maybe I just missed a feature of Rust that makes this nicer.

Before this I tried having 1 struct and using the T to add different implementations so you get something like

pub struct Builder<T>(PhantomData);
impl Builder<()> {
    fn new()-> Self {
       Self(PhantdomData) 
   }
   fn with::<T>(self) -> Builder<(T)>
   where (T):Query
   {
        Builder<(T)>(PhantomData)
    }
}
imp<T>  Builder<(T)> {
   fn with::<N>(self) -> Builder<(T,N)>
   where (T,N):Query
   {
        Builder<(T,N)>(PhantomData)
    }
}

This however fails because the (T) is the same as T and from there it conflicts with every type.

@olefasting
Copy link
Member

It's been maybe 8-9 years since I touched lua last time, so I'm a bit out of the loop :P

The former doesn't seem optimal, though, as it will require implementations on the builder for each component type (?). As any type can be a component, this might be a hassle, so we should probably try to avoid that

@lenscas
Copy link
Author

lenscas commented Feb 21, 2022

It's been maybe 8-9 years since I touched lua last time, so I'm a bit out of the loop :P

The former doesn't seem optimal, though, as it will require implementations on the builder for each component type (?). As any type can be a component, this might be a hassle, so we should probably try to avoid that

Sadly, yes that would require a with_ for every type that needs to be queried. Ideally, there are values that lua could use as types so instead of builder:with_player() it becomes builder:with(FishFight.Player)

There are however 2 problems with that:
First: This still doesn't allow lua to query every type, as it is still limited to types that implement the correct interface to make this happen.

Second: hecs ONLY cares about types. So we end up asking Rust to create a new type based on the value that it got from Lua. Not exactly something that Rust likes...

and... I just got an idea :)
time to play a bit with code. I don't think the code will get any prettier though :(

edit: object safety makes that idea not work. I'm going to think about the problem some more however I think that the best solution would be if hecs had support for dynamic queries. There are 2 PR's on hecs for that already

Ralith/hecs#202 and Ralith/hecs#196
I'm going over those 2 tomorrow. I don't think I have what it takes to get them merged with hecs, but maybe they give me some inspiration on how I can work around the lack of dynamic queries for the time being 🤷

@olefasting
Copy link
Member

olefasting commented Feb 21, 2022

Would this be implementations in rust or in LUA? If in LUA it is at least possible to consider it, worst case. If not, it is a huge problem, as we want it to be possible to define types in LUA and use them as components with hecs, somehow

@lenscas
Copy link
Author

lenscas commented Feb 21, 2022

Would this be implementations in rust or in LUA? If in LUA it is at least possible to consider it, worst case. If not, it is a huge problem, as we want it to be possible to define types in LUA and use them as components with hecs, somehow

Lua doesn't allow you to define new types from withing lua. If you know what you are doing you can fake it using metatables but Rust would need to have access to their actual values to see a difference between these "types".

So, it might be possible to get it working if hecs has dynamic queries. But, without that I don't see how any scripting language is going to be able to create types that work with hecs as the types defined by said language don't exist as far as Rust knows.

edit: to explain it a bit better, just in case I am wrong. If scripting language X creates a new Type (lets say, ScriptedPlayer). In order for this type to be given to Rust so it can be put in the ecs it needs to be transformed into something Rust can understand. Which would likely involve it being turned into another, more generic type.

You can query the ecs for this more generic type, of course but that gives you every component of this more generic type. Not just ScriptedPlayer.

@olefasting
Copy link
Member

olefasting commented Feb 21, 2022

Yeah, you are right about that, of course. We will probably have to create some sort of wrapper component for these cases that can hold a table of data, or something. We will probably just create a ModData component that holds a table of enums, much like is done for custom data in our maps. Mods will be restricted to common, basic data types in that component, but that shouldn't really be a problem....

In that case, it wouldn't be a crisis to require implementations for all component types on the builder but it would be nice to avoid it, if we can. We could also just make an attribute macro for it that can be set on all component structs, and do it manually for std types

@lenscas
Copy link
Author

lenscas commented Feb 22, 2022

Yeah, you are right about that, of course. We will probably have to create some sort of wrapper component for these cases that can hold a table of data, or something. We will probably just create a ModData component that holds a table of enums, much like is done for custom data in our maps. Mods will be restricted to common, basic data types in that component, but that shouldn't really be a problem....

mlua can work with serde. So, if the wrapper just wraps around serde_json::Value or similar then mods can use whatever data and structure they like. I am not sure about the performance implications of that though. You can also directly wrap Table or mlua::Value but there are lifetimes attached to those. So, that might prove to be a headache.

Anyway, my current plan:

Go over those PR's and comments to see if there is something from them that can be used to get what I want working. However, I kind of want to get something working before spending too much time on this part as I am sure there will be other roadblocks ahead and I rather ram into them with a prototype that I spent a few hours on than a fully thought out solution that costed me a few days 😅

From there, it is always possible to revamp the API to something more suitable and nicer to work with. Especially as the internals of this part probably change if/when hecs get dynamic query support anyway.

@olefasting
Copy link
Member

Awesome!

Zac8668 and others added 9 commits February 25, 2022 00:50
* First logic

* Solved warnings

* Added Autotiling

It is almost done, just need to account for tile subdivisions, and add autotile mask bitmasks as an attribute.

* Fixed formatting

* Added tileset bitmasks field

Added the bitmasks field for the tileset and some support to  tile subdivisions

* Minor changes

* Added random choosing
* Include licenses folder in release archives

* Include the main license file in releases
* Create PlayerLionfishy(96x80).png

* Add Lionfishy character

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
* Create PlayerCatfishy(96x80).png

* new player

i missed the name

* Add Catsharky character

* Rename Catsharky to Catty

Co-authored-by: Orhun Parmaksız <orhunparmaksiz@gmail.com>
@lenscas
Copy link
Author

lenscas commented Mar 7, 2022

Switched to the forks from https://github.com/sdleffler/hv-dev/

Right now there is still no way for the lua VM to get the world I believe. Need to wrap it and then that part should be pretty much done.

For queries, there is some boilerplate needed to get types working. However that should be easy enough to put in a macro_rules macro. https://github.com/fishfight/FishFight/blob/1bb36fe16e65244b0741fcbaba5be0db039e6bf1/core/src/test.rs#L12 is a file in which I experimented a bit with getting lua and hecs to work. Here you can see this boilerplate.

Other improvements that still need to be done:

  • The lua stuff needs to be moved to the core crate
  • Lua needs to work with the mod_id, rather than the folder for the require_from function.
  • A LOT more of the systemFN's need to be exposed to lua. Right now, only 1 kind gets called.
  • The structure that lua uses to call these functions need to be improved. Right now, it loops over every mod instead of only over the functions that need to be called and the order is not stable
  • Right now, tealr is being side stepped. Obviously this needs to change as well so documentation and the like can be generated.
  • Probably lots of other stuff.

@lenscas
Copy link
Author

lenscas commented Mar 9, 2022

the current type bodies for hv_ecs::{World,Entity, etc} is.. done a bit hacky and not complete. However it is a start.

@erlend-sh
Copy link
Member

Due to the #466 Bevy Rewrite this work is defunct, but the contribution is recognized and remains appreciated! 🙏

@erlend-sh erlend-sh closed this Nov 20, 2022
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 this pull request may close these issues.

6 participants