~gpcf/advtrains-devel

3 3

[Patch] Doors improvements

Details
Message ID
<CAAvRrcGepr=Gjvi3Sd442oU=pE==KRFatZ+GE78sOOhtxAmVvA@mail.gmail.com>
DKIM signature
missing
Download raw message
Hey,

Recently I needed more doors utils for ingame coding with advtrains,
so I added two "new" features to advtrains, and fixed an inconsistency
(not a crashing bug) to advtrains. Let me explain :
1. Add `atc_doors` command. It runs on Lua ATC rail. I used the
following syntax : `{ L = bool, R = bool }`. It is for upcoming
support (if added a day or not) of both-side opening.
2. Add `toggle_doors_on_node` callback. It checks if doors booleans
have changed. If so, it runs `on_train_toggle_doors`. On a LuaATC
rail, it returns the `doors` event type. However, you should look at
the `te_register_on_update()` function because I'm not sure iterating
through the `path` table of every track index is the best solution, it
may generate lag. But, it still removes some `interrupt` cycles at
stations for checking state of `atc_doors` (notably for automatic
doors stations).
3. It just has the `D` command at the beginning of ATC rules on a
Station/Stop track to wait door delay value at both entering/leaving.

I send 3 different patches in case you think a certain commit should
be merged but no others (I hope it exported GPG signing).

Thanks for reading me, and thanks also to Blockhead who helped me and
gave me some guidelines =)

NB: I resent this mail because it told me I was using HTML.

-- Athozus
Details
Message ID
<CAAvRrcHJ2XcOGmxtmWf_VF3gn+dKxUWovgRQFvOsbP2tv1OqPA@mail.gmail.com>
In-Reply-To
<CAAvRrcGepr=Gjvi3Sd442oU=pE==KRFatZ+GE78sOOhtxAmVvA@mail.gmail.com> (view parent)
DKIM signature
missing
Download raw message
Oh, and due to that error, I forgot to send the patches.

Sorry about that, here they are.

Le dim. 22 oct. 2023 à 00:25, Athozus <athozus@gmail.com> a écrit :
>
> Hey,
>
> Recently I needed more doors utils for ingame coding with advtrains,
> so I added two "new" features to advtrains, and fixed an inconsistency
> (not a crashing bug) to advtrains. Let me explain :
> 1. Add `atc_doors` command. It runs on Lua ATC rail. I used the
> following syntax : `{ L = bool, R = bool }`. It is for upcoming
> support (if added a day or not) of both-side opening.
> 2. Add `toggle_doors_on_node` callback. It checks if doors booleans
> have changed. If so, it runs `on_train_toggle_doors`. On a LuaATC
> rail, it returns the `doors` event type. However, you should look at
> the `te_register_on_update()` function because I'm not sure iterating
> through the `path` table of every track index is the best solution, it
> may generate lag. But, it still removes some `interrupt` cycles at
> stations for checking state of `atc_doors` (notably for automatic
> doors stations).
> 3. It just has the `D` command at the beginning of ATC rules on a
> Station/Stop track to wait door delay value at both entering/leaving.
>
> I send 3 different patches in case you think a certain commit should
> be merged but no others (I hope it exported GPG signing).
>
> Thanks for reading me, and thanks also to Blockhead who helped me and
> gave me some guidelines =)
>
> NB: I resent this mail because it told me I was using HTML.
>
> -- Athozus
Details
Message ID
<4347e39c-f991-4cd5-a009-e8fab5094d20@forksworld.de>
In-Reply-To
<CAAvRrcHJ2XcOGmxtmWf_VF3gn+dKxUWovgRQFvOsbP2tv1OqPA@mail.gmail.com> (view parent)
DKIM signature
missing
Download raw message
On 22/10/2023 00:27, Athozus wrote:
> Oh, and due to that error, I forgot to send the patches.
> 
> Sorry about that, here they are.

I would suggest using git-send-email[1] if possible. It sends the 
patches as the content of the email and allows people to quote snippets 
from the patch directly; you will need to send multiple emails with it, 
though, but then people know the order in which the patches should be 
applied, if this is done correctly.

[1] https://git-send-email.io/

The patch itself looks good with some (IMO) minor things that I would 
like to point out.


In d23fbdb:
diff --git a/advtrains_luaautomation/README.md 
b/advtrains_luaautomation/README.md
index a885075..b67369f 100644
--- a/advtrains_luaautomation/README.md
+++ b/advtrains_luaautomation/README.md
@@ -227,6 +227,9 @@ In addition to the above environment functions, the 
following functions are avai
   - `atc_speed`
  	Speed of the train, or nil if there is no train.

+ - `atc_doors`
+	Doors opened of the train, `{L = bool, R = bool}` syntax, or nil if 
there is no train.
+
   - `atc_set_text_outside(text)`
  	Set text shown on the outside of the train. Pass nil to show no text. 
`text` must be a string.

diff --git a/advtrains_luaautomation/atc_rail.lua 
b/advtrains_luaautomation/atc_rail.lua
index aac11f0..ac16128 100755
--- a/advtrains_luaautomation/atc_rail.lua
+++ b/advtrains_luaautomation/atc_rail.lua
@@ -27,11 +27,14 @@ function r.fire_event(pos, evtdata, appr_internal)
  	local train_id = evtdata._train_id
  	local atc_arrow = evtdata._train_arrow
  	local train, tvel
+	local tdoors = { L = false, R = false }
  	
  	if train_id then

Unfortunately the documentation and the implementation are inconsistent. 
The code here sets atc_doors to {L = false, R = false} when there is no 
train. This behavior can be tested with a LuaATC track:

if event.type == "train" and atc_arrow then
   atc_send("SM")
   interrupt(5)
end

local tp = type(atc_doors)
print(tp)
if tp == "table" then
   for k, v in pairs(atc_doors) do
     print(("%s %s"):format(k, v))
   end
end

IMO atc_doors should be set to nil when there is no train, as for other 
values.


In ccc09b7:

  advtrains.te_register_on_new_path(function(id, train)
  	train.tnc = {
  		old_index = atround(train.index),
  		old_end_index = atround(train.end_index),
+		old_doors = { L = train.door_open == -1, R = train.door_open == 1 }
  	}
  	--atdebug(id,"tnc init",train.index,train.end_index)
  end)
@@ -923,13 +939,21 @@ end)
  advtrains.te_register_on_update(function(id, train)
  	local new_index = atround(train.index)
  	local new_end_index = atround(train.end_index)
+	local new_doors = { L = train.door_open == -1, R = train.door_open == 1 }
  	local old_index = train.tnc.old_index
  	local old_end_index = train.tnc.old_end_index
+	local old_doors = train.tnc.old_doors
  	while old_index < new_index do
  		old_index = old_index + 1
  		local pos = 
advtrains.round_vector_floor_y(advtrains.path_get(train,old_index))
  		tnc_call_enter_callback(pos, id, train, old_index)
  	end
+	if new_doors.L ~= old_doors.L or new_doors.R ~= old_doors.R then
+		for track_index, _ in pairs(train.path) do
+			local pos = 
advtrains.round_vector_floor_y(advtrains.path_get(train,track_index))
+			tnc_call_toggle_doors_callback(pos, id, train, track_index)
+		end
+	end

IMO it is not necessary here to convert the door information to convert 
the door information and then compare them. Is there any reason to not 
compare the train.door_open values directly?
Details
Message ID
<20240801224939.47bd9f51@BARIUM>
In-Reply-To
<4347e39c-f991-4cd5-a009-e8fab5094d20@forksworld.de> (view parent)
DKIM signature
pass
Download raw message
Hi Athozus,

I know it has been some time. Maybe I should check mails on this address more often, but I have recently reinstalled my laptop and had so far not setup the email client.

Regarding the door patches, I need to check them in more detail. But I would like to finish my work on the route programming (route_prog_rework) before I do such things.
I hope you are not too upset because of this. Thanks for understanding.

Regards,
orwell

Am Fri, 29 Dec 2023 12:29:00 +0100
schrieb "Y. Wang" <yw05@forksworld.de>:

> On 22/10/2023 00:27, Athozus wrote:
> > Oh, and due to that error, I forgot to send the patches.
> > 
> > Sorry about that, here they are.  
> 
> I would suggest using git-send-email[1] if possible. It sends the 
> patches as the content of the email and allows people to quote snippets 
> from the patch directly; you will need to send multiple emails with it, 
> though, but then people know the order in which the patches should be 
> applied, if this is done correctly.
> 
> [1] https://git-send-email.io/
> 
> The patch itself looks good with some (IMO) minor things that I would 
> like to point out.
> 
> 
> In d23fbdb:
> diff --git a/advtrains_luaautomation/README.md 
> b/advtrains_luaautomation/README.md
> index a885075..b67369f 100644
> --- a/advtrains_luaautomation/README.md
> +++ b/advtrains_luaautomation/README.md
> @@ -227,6 +227,9 @@ In addition to the above environment functions, the 
> following functions are avai
>    - `atc_speed`
>   	Speed of the train, or nil if there is no train.
> 
> + - `atc_doors`
> +	Doors opened of the train, `{L = bool, R = bool}` syntax, or nil if 
> there is no train.
> +
>    - `atc_set_text_outside(text)`
>   	Set text shown on the outside of the train. Pass nil to show no text. 
> `text` must be a string.
> 
> diff --git a/advtrains_luaautomation/atc_rail.lua 
> b/advtrains_luaautomation/atc_rail.lua
> index aac11f0..ac16128 100755
> --- a/advtrains_luaautomation/atc_rail.lua
> +++ b/advtrains_luaautomation/atc_rail.lua
> @@ -27,11 +27,14 @@ function r.fire_event(pos, evtdata, appr_internal)
>   	local train_id = evtdata._train_id
>   	local atc_arrow = evtdata._train_arrow
>   	local train, tvel
> +	local tdoors = { L = false, R = false }
>   	
>   	if train_id then
> 
> Unfortunately the documentation and the implementation are inconsistent. 
> The code here sets atc_doors to {L = false, R = false} when there is no 
> train. This behavior can be tested with a LuaATC track:
> 
> if event.type == "train" and atc_arrow then
>    atc_send("SM")
>    interrupt(5)
> end
> 
> local tp = type(atc_doors)
> print(tp)
> if tp == "table" then
>    for k, v in pairs(atc_doors) do
>      print(("%s %s"):format(k, v))
>    end
> end
> 
> IMO atc_doors should be set to nil when there is no train, as for other 
> values.
> 
> 
> In ccc09b7:
> 
>   advtrains.te_register_on_new_path(function(id, train)
>   	train.tnc = {
>   		old_index = atround(train.index),
>   		old_end_index = atround(train.end_index),
> +		old_doors = { L = train.door_open == -1, R = train.door_open == 1 }
>   	}
>   	--atdebug(id,"tnc init",train.index,train.end_index)
>   end)
> @@ -923,13 +939,21 @@ end)
>   advtrains.te_register_on_update(function(id, train)
>   	local new_index = atround(train.index)
>   	local new_end_index = atround(train.end_index)
> +	local new_doors = { L = train.door_open == -1, R = train.door_open == 1 }
>   	local old_index = train.tnc.old_index
>   	local old_end_index = train.tnc.old_end_index
> +	local old_doors = train.tnc.old_doors
>   	while old_index < new_index do
>   		old_index = old_index + 1
>   		local pos = 
> advtrains.round_vector_floor_y(advtrains.path_get(train,old_index))
>   		tnc_call_enter_callback(pos, id, train, old_index)
>   	end
> +	if new_doors.L ~= old_doors.L or new_doors.R ~= old_doors.R then
> +		for track_index, _ in pairs(train.path) do
> +			local pos = 
> advtrains.round_vector_floor_y(advtrains.path_get(train,track_index))
> +			tnc_call_toggle_doors_callback(pos, id, train, track_index)
> +		end
> +	end
> 
> IMO it is not necessary here to convert the door information to convert 
> the door information and then compare them. Is there any reason to not 
> compare the train.door_open values directly?
Reply to thread Export thread (mbox)