From Y. Wang to ~gpcf/advtrains-devel
> +function advtrains.next_wagon_entity_in_train(train, i) > + local wagon_id = train.trainparts[i + 1] > + if wagon_id then > + local wagon = advtrains.get_wagon_entity(wagon_id) > + return wagon and i + 1 or nil, wagon I don't think this is correct: if you reach wagon == nil you effectively end the iteration here even if wagons further down in train.trainparts do have luaentities. I think we probably want something like this: for idx = i+1, #train.trainparts do local wagon = advtrains.get_wagon_entity(train.trainparts[idx])
From Y. Wang to ~gpcf/advtrains-devel
As it turns out, not fully testing new features is not necessarily a
good idea ...
This patch follows up 1F616EMO's patch by
* Making get_wagon_prototype return the resolved alias,
* Handling recursive wagon alises (in particular, loops), and
* Adding (partial) unittest for the wagon aliasing system
How to test:
* In a world with both advtrains_train_subway and advtrains_train_japan
enabled, place a subway wagon, a Japanese engine, and a regular
Japanese wagon.
* Add the test mod to the world; do NOT remove advtrains_train_japan.
* Restart the world. Notice that the Japanese wagons still appear as
[message trimmed]
From Y. Wang to ~gpcf/advtrains-devel
Looks good beside some minor things. > +function advtrains.next_wagon_entity_in_train(train, i) > + local wagon_id = train[i + 1] Shouldn't this be train.trainparts? > + if wagon_id then > + return i + 1, advtrains.get_wagon_entity(wagon_id) It would IMO be better to have a nil check here since other places use the entity directly without nil checks.
From Y. Wang to ~gpcf/advtrains-devel
This patch avoids sending the driver HUD if it contains the same text that was previously sent. --- advtrains/trainhud.lua | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/advtrains/trainhud.lua b/advtrains/trainhud.lua index b2744ad..f9f4876 100644 --- a/advtrains/trainhud.lua +++ b/advtrains/trainhud.lua @@ -103,18 +103,20 @@ function advtrains.set_trainhud(name, text, driver) if not player then return [message trimmed]
From Y. Wang to ~gpcf/advtrains-devel
Tested and (mostly) works: * Showing and handling boardcom mostly works; exception: wagon properties form does not show up (see code review below). * Showing and handling the wagon formspec (i.e. right-clicking the wagon from the inside) works. * Showing and handling the wagon properties formspec works. * Showing and handling the wagon inventory form works. * Players are reattached to the wagon when joining. * Players are removed from the wagon when dying. (Also: this has turned into a small refactor for wagon code now.) > - elseif fields["wgprp"..i] then > - for _,wagon in pairs(minetest.luaentities) do
From Y. Wang to ~gpcf/advtrains-devel
> @@ -142,12 +142,11 @@ minetest.register_on_joinplayer(function(player) > local pname = player:get_player_name() > local id=advtrains.player_to_train_mapping[pname] > if id then > - for _,wagon in pairs(minetest.luaentities) do > - if wagon.is_wagon and wagon.initialized and wagon.id then > - local wdata = advtrains.wagons[wagon.id] > - if wdata and wdata.train_id == id then > - wagon:reattach_all() > - end > + local wagon = advtrains.get_wagon_by_id(id, true) Is get_wagon_by_id(id, true) correct here? The id variables seems to be the train ID (hence the later wagon.train_id == id check).
From Y. Wang to ~gpcf/advtrains-devel
Nice. I have some minor suggestions though: On 06/09/2024 15:27, 1F616EMO wrote: > diff --git a/advtrains/trainlogic.lua b/advtrains/trainlogic.lua > index f136577..81c4622 100644 > --- a/advtrains/trainlogic.lua > +++ b/advtrains/trainlogic.lua > @@ -142,12 +142,11 @@ minetest.register_on_joinplayer(function(player) > local pname = player:get_player_name() > local id=advtrains.player_to_train_mapping[pname] > if id then > - for _,wagon in pairs(minetest.luaentities) do > - if wagon.is_wagon and wagon.initialized and wagon.id then > - local wdata = advtrains.wagons[wagon.id]
From Y. Wang to ~gpcf/advtrains-devel
I would suggest additionally refactoring this a bit so that we can have less nesting. In particular, it would be nice to have (1) a function that returns an iterator for wagon entities and (2) a function that returns the wagon entity with the given ID. On 04/09/2024 16:44, 1F616EMO wrote: > @@ -1157,21 +1157,24 @@ minetest.register_on_player_receive_fields(function(player, formname, fields) > for _,wagon in pairs(minetest.luaentities) do > if wagon.is_wagon and wagon.initialized and wagon.id==uid then > local data = advtrains.wagons[wagon.id] > - if fields.inv then > - if wagon.has_inventory and wagon.get_inventory_formspec then > - minetest.show_formspec(player:get_player_name(), "advtrains_inv_"..uid, wagon:get_inventory_formspec(player:get_player_name(), make_inv_name(uid))) > - end
From Y. Wang to ~gpcf/advtrains-devel
* Fix broken link to RWT API documentation. * w_speed is no longer relevant since the 2021 new-ks update. --- advtrains_luaautomation/README.md | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/advtrains_luaautomation/README.md b/advtrains_luaautomation/README.md index f98f7a0..f285d0f 100644 --- a/advtrains_luaautomation/README.md +++ b/advtrains_luaautomation/README.md @@ -145,22 +145,16 @@ asp = { -- the character of call_on and dead_end is purely informative call_on = <boolean>, -- Call-on route, expect train in track ahead (not implemented yet) dead_end = <boolean>, -- Route ends on a dead end (e.g. bumper) (not implemented yet) [message trimmed]
From Y. Wang to ~gpcf/advtrains-devel
Looks good. Some minor suggestions: On 31/08/2024 16:58, 1F616EMO wrote: > +advtrains.wagon_prototypes = setmetatable({}, { > + __index = function(t, k) > + local rtn_val = rawget(t, k) > + if rtn_val ~= nil then > + return rtn_val > + end This condition is not needed. Lua uses the __index metamethod only if the table does not contain a certain key, so you can assume here that rawget(t, k) is nil.