Fluorohydride / ygopro-core

ygopro script engine.
MIT License
327 stars 134 forks source link

use ref in PROCESSOR_SELECT_SYNCHRO #606

Closed salix5 closed 3 months ago

salix5 commented 4 months ago

Add interpreter::check_filter

check_matching is replaced by check_filter, which takes lua_State* as 1st parameter.

This function is usually called from Lua functions like:

int32 scriptlib::duel_select_matching_cards(lua_State *L);

The arguments are on the stack of L, so the function indices are also the index of L.

Add ptr3, ptr4 to processor_unit

d5accce Avoid unnecessary pointer casting.

Push pcard in check_tuner_material

A new parameter scard is added to Card.IsNotTuner in 3c46099, and the new parameter is pushed onto the stack before every function call to check_tuner_material orcheck_synchro_material. It will make the stack tracing very hard.

0101614 Because we have the pointer to L now, we can push the synchro monster pcard is onto the stack of L before check_filter. It will make the stack tracing easier.

Always take mg first

c752dee93a75c440330c9c2f53bd430f478e9c8e Before: In duel_select_synchro_material, smat will be ignored if mg is given. In duel_check_synchro_material, smat will not be ignored if mg is given.

Now: smat will always be ignored if mg is given.

Remove lua_pop in synchro procedures

https://github.com/Fluorohydride/ygopro/issues/2407 I agree that passing Lua functions by pushing onto the stack is a bad idea too.

Problems in https://github.com/Fluorohydride/ygopro/issues/2406

https://www.lua.org/manual/5.4/manual.html#lua_status You can call functions only in threads with status LUA_OK. You can resume threads with status LUA_OK (to start a new coroutine) or LUA_YIELD (to resume a coroutine).

Before #582 In order to reuse the arguments on the stack, ygopro used the yielded thread to call new functions. It is not allowed by Lua, so the assertion of LUA_USE_APICHECK will fail.

After #582 Now we restore current_state after lua_resume returns. The Lua treads are: lua_state rthread (The arguments of coroutine are here)

So we have to use lua_xmove to move the arguments to another stack.

Problems in https://github.com/Fluorohydride/ygopro/issues/2407

Pushing the arguments on the stack of rthread and moving to another stack is harmful. It will make the stack tracing hard as hell.

The heap corruption still happens after #582 . I think it is caused by the same problems in https://github.com/Fluorohydride/ygopro/issues/2407

Solution

If we want to use Lua functions or any objects after the coroutine yields: We can use reference in Lua registry instead.

If we want to call a Lua filter outside the Lua functions: We can use interpreter::check_condition instead.

Now the functions filter1 and filter2 are passed to select_synchro_material by the reference in registry. We don't need to use PARAM_TYPE_INDEX anymore. fix https://github.com/Fluorohydride/ygopro/issues/2407

remove Group.ForEach

https://github.com/edo9300/ygopro-core/commit/de60f7982a2a2faae24d5fb03f36b4eb14b0b6c8 I have reached the same conclusion. Group.ForEach is not used in ygopro-scripts now. It can (and should) be replaced by normal for-loop in Lua.

PARAM_TYPE_INDEX is not used now.


https://github.com/Fluorohydride/ygopro/issues/2406 https://github.com/Fluorohydride/ygopro/issues/2407 Lua線程 lua_state(主線程) rthread (呼叫coroutine使用)

原本的傳遞參數方式 函數f1 push到rthread的堆疊 lua_xmove移動到主線程 傳遞f1在主線程堆疊的index 手動從主線程的堆疊pop

這是 @mercury233 記錄在2407的問題 這會讓堆疊的管理變得非常麻煩並且容易出錯 我認為同步召喚有時會產生的heap corruption的主要原因是這個

現在改成 傳遞函數都用registry reference 這是每個線程共通的數值 由於沒有手動push這步 也就不再需要手動pop

Group.ForEach 這個函數會有和2407相同的問題 現有的腳本沒有使用 並且他的功能可以由普通的Lua迴圈完成 因此移除這個函數

@mercury233 @purerosefallen

purerosefallen commented 4 months ago

I would consider this in the next update, because 3 days are not enough for reviewing and testing.