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

Tracking issue for old broken E2s: BFW / ALXPC #2841

Open
Vurv78 opened this issue Nov 12, 2023 · 37 comments
Open

Tracking issue for old broken E2s: BFW / ALXPC #2841

Vurv78 opened this issue Nov 12, 2023 · 37 comments
Labels

Comments

@Vurv78
Copy link
Member

Vurv78 commented Nov 12, 2023

This is just an issue to track any progress on fixing E2s unintentionally broken by changes to E2

Largest chip seems to be this BFW, and apparently the ALX E2 PC is also broken.

Currently identified inconsistencies:

  • Removal of wire_expression2_delta slightly broke one public chip (one leg sticks up), hoping this is rare enough. That code really never should've worked in the first place. But if more chips are found that rely on this behavior, just == will be reverted to old behavior.
@Vurv78 Vurv78 changed the title Tracking issue for old broken E2s: BFW / AlyxPC Tracking issue for old broken E2s: BFW / ALXPC Nov 12, 2023
@Denneisk
Copy link
Member

Denneisk commented Nov 12, 2023

ALX PC OK apparently now...?

  • Has a bad function in alx_pc/hdd/wm1_fs.txt (unenforced return)
  • Its HDDs (contraption) don't seem to be recognized by the computer(despite fixing above)
  • (Not an issue in strictless) NULL entity for runOnKeys in alxos/_main_.txt followed by code with side-effects

BFW

  • Error seems to be somewhere beginning line 323. HCP variable appears to be used for PID but its value may be dirty.
  • Removing the hip force makes Line 337 suspect. It seems the feet are the source of the errant force, now it's just knowing why it's erring.

Vurv78 added a commit to Vurv78/wiremod_e2_os that referenced this issue Nov 12, 2023
New change to function ensures they return something at compile time for performance. This function was missing a return in case all the if statements failed

wiremod/wire#2841
@CheezusChrust
Copy link
Contributor

CheezusChrust commented Nov 15, 2023

Testing on the latest version of wiremod, this is causing some of my old public E2s to stop working. Narrowed it down to a setup like this:

@name bruh
@strict

if(first()) {
    timer("piss", 0)
    
    Var = 0
}

if(clk("piss")) {
    Var++

    print($Var)
    
    timer("piss", 1000)
}

I would expect it to be printing 1 every second, instead it just seems to be printing the current value of Var.

@Denneisk
Copy link
Member

Denneisk commented Nov 15, 2023

Testing on the latest version of wiremod, this is causing some of my old public E2s to stop working. Narrowed it down to a setup like this:

I would expect it to be printing near 1 every second, instead it just seems to be printing the current value of curtime.

I don't seem to understand. Was this using $? Curtime = curtime() isn't exactly vague.

@CheezusChrust
Copy link
Contributor

CheezusChrust commented Nov 15, 2023

Sorry, edited, check now

@M-dot-Core
Copy link

M-dot-Core commented Nov 15, 2023

Hi, the recent E2 update changed Delta in such a way that it no longer provides damping to my hovering vehicles, resulting in bouncing and crashing into every type of terrain it encounters.

An example being:

@inputs Base:entity
@outputs Vel Vel_Delta
@persist Vel_Delta

interval(30)

Vel = Base:vel():z()

Vel_Delta = $Vel

Where Vel_Delta is supposed to measure Vel's difference between the previous and current interval, or change in distance from the ground in case of a ranger, and gets fed into applyForce.

Currently, $ doesn't do anything at all, as if the person forgot to put the required value in @persist.

@Denneisk Denneisk mentioned this issue Nov 15, 2023
@AlexALX
Copy link
Contributor

AlexALX commented Nov 16, 2023

Since there is "dedicated" issue for my old alxpc, i did merged recent changes and having one more issue related to EGP.

All menus are shift by some reason. What other changes broke this?

изображение
изображение

Also question about string calls been deprecated:
изображение

Lambdas are NOT same as string call. you can't just store function name in string and call function IF this will be removed in future. My whole PC based on this thing and would be impossible to work without string calls. Many programming languages have string calls so not sure why you want to deprecate it in first place. its wrong behavior. String call can store ANY function name and then can be called (so they are dynamic and can be changed on fly), while Lambdas are fixed for only one function (correct me if i'm wrong).

@Vurv78
Copy link
Member Author

Vurv78 commented Nov 16, 2023

Since there is "dedicated" issue for my old alxpc, i did merged recent changes and having one more issue related to EGP.

All menus are shift by some reason. What other changes broke this?

Probably from @Denneisk's EGP rewrites, would be worth trying to reproduce to him.

Also question about string calls been deprecated: изображение

Lambdas are NOT same as string call. you can't just store function name in string and call function IF this will be removed in future.

The message says they will be an error on @strict. Your PC likely hasn't worked since @strict was first implemented. So this is no problem for you. I will maintain backwards compatibility for chips without @strict as usual.

Lambdas are the same though. They are function references. At the moment there's no functionality to reference builtin functions, but that isn't hard to implement, at least for freestanding functions. Methods would be annoying indeed so I guess I would have to essentially reimplement stringcalls with let MyLambda = delegate("foo(e:)").

Once again reiterating they aren't being removed. This is just a warning to let people know in advance that there is a significantly better system now they can start migrating to.

Many programming languages have string calls so not sure why you want to deprecate it in first place. its wrong behavior.

Many programming languages like? Only programming language that I know has string calls is PHP. And everyone knows PHP is hot garbage. Also FYI, PHP is also replacing string calls with a delegate / lambda system, exactly like what I'm doing with E2. Even if a bunch of languages had string calls they are just really bad. Delegate system / lambdas are much better.

String call can store ANY function name and then can be called (so they are dynamic), while Lambdas are fixed for only one function (correct me if i'm wrong).

You are indeed wrong, the entire point of a lambda is that they can be replaced at runtime..

let X = function() {}
X = function() {}

Better yet, E2's lambdas, not dissimilar to tables, are type erased at compile time. So you could even replace a function that takes a number with one that takes a string.

@AlexALX
Copy link
Contributor

AlexALX commented Nov 16, 2023

Probably from @Denneisk's EGP rewrites, would be worth trying to reproduce to him.

It might be related to parent system or something. Previously if we set parent then coordinates are going from parent not global ones, or something like this. but now it goes totally off. Need to write some example then later... because no sense to push update if egp is broken

You are indeed wrong, the entire point of a lambda is that they can be replaced at runtime..

well, by your example it means i need replace it WITH function, while callable string only have name of function.

so for example if i need call some function when receive it as string argument - it will be tons of IF rules then with lambda variables instead of just FuncNameVar(), or at least i don't see how i can call needed function.

here is few examples:

FuncNameVar = "someTestFunc"

FuncNameVar()

FuncNameVar = "someOtherTestFunc"

FuncNameVar()

with lambda it will need to pass func reference, but if its cross-chip communication (like i have dsSendDirect), you don't have that function in CPU for example, but have only on GPU side, or etc. So i can't pass func reference in this case and on other chip will be then something like this:


FuncName = dsGetString();

if (FuncName=="someTestFunc") {
someTestFunc()
} else if (FuncName=="someOtherTestFunc") {
someOtherTestFunc()
}

Except there will be way to get func reference from string. like this

FuncName = dsGetString()
Func = get_func_reference(FuncName)
Func()

@Denneisk
Copy link
Member

Denneisk commented Nov 16, 2023

Since there is "dedicated" issue for my old alxpc, i did merged recent changes and having one more issue related to EGP.

All menus are shift by some reason. What other changes broke this?

This could be part of the fix for parented objects not being drawn to the top left after being parented. If your code is manually correcting for egpDrawTopLeft not having parented objects also draw in the top left corner of the parent object, then you'll have to revert that manual fix as it's a feature now.

So i can't pass func reference in this case and on other chip will be then something like this:

You can have a table of functions by name so you can do

const Functions = table(
    "someTestFunc" = SomeTestFunc,
    "someOtherTestFunc" = SomeOtherTestFunc
)

And then use that when trying to "string call".

@AlexALX
Copy link
Contributor

AlexALX commented Nov 16, 2023

You can have a table of functions by name so you can do

now each new function needs to be added to table, and then having huge table with 100+ functions in list. but yeah thats possible, but can't be made as simple as it is with callable string. anyway will back to egp when will have example, do i need open new issue for that?

@Denneisk
Copy link
Member

back to egp when will have example, do i need open new issue for that?

I don't intend on reverting EGP parenting since it was bad design before. If you want to keep it the old way you'll have to disable egpDrawTopLeft before creating parented objects, or change your code to not have to move itself.

Line 201 of alx_pc/gpu/alxos/installos.txt

Spos = vec2(Size[1]/2*(-1)+10,Size[2]/2*(-1))

This line for example can be simplified to just

Spos = vec2(10, 0)

@AlexALX
Copy link
Contributor

AlexALX commented Nov 16, 2023

I don't intend on reverting EGP parenting since it was bad design before. If you want to keep it the old way you'll have to disable egpDrawTopLeft before creating parented objects, or change your code to not have to move itself.

why its bad design? relative to parent is as simple as css position: relative / absolute; or i'm already forgot something. i'll check your example later. i just wanted to know what exactly changed in recent updates. thanks

@Denneisk
Copy link
Member

Denneisk commented Nov 16, 2023

why its bad design?

The function name implies that everything should draw from the top left corner based on the parent's top left corner. When objects are parented, the old functionality was that parented objects wouldn't draw from the top left. This was changed so that they do. It's still a bit misleading in that stuff like circles and polys don't offset based on their size but I was worried that would cause too much confusion.

I could try to see about making something for relative position on parented objects.

i just wanted to know what exactly changed in recent updates.

These are all (or most) of the functionality-changing EGP PRs that I've made
#2844
#2646
#2731
#2624
#2595 (Changes parenting behavior)
#2588

@AlexALX
Copy link
Contributor

AlexALX commented Nov 18, 2023

Ok i got main change of this - now parented text and other elements in egpDrawTopLeft mode draws relative to parent topleft edge, not center like it was before (so probably its right behavior).

I did successfully fixed menu positions with minor changes, but since i have hardcoded positions for all dialogs modals (those with text inputs and ok/cancel btns etc), this change requires me to update ALL positions of ALL dialogs in ALL programs i have. This is huge amount of work... set egpDrawTopLeft(0) and then back to 1 seems to not work as well properly.
изображение

Not sure what to do, since i never expected to change in how egpPos works when wrote my script... rewriting it will take months to do since i'm lack of free time. Guess i have no choice so will slowly redo it probably...

@AlexALX
Copy link
Contributor

AlexALX commented Nov 18, 2023

Oh damn, i have much more issues, when trying to create file there is some error in e2 and hdd controller crashes:

sv: Expression 2 (HDD Controller): Runtime error 'Mismatching return types. Got n, expected void' at line 225, char 9

Guess some function doing this... and i guess there will be tons of this. Basically i guess can RIP whole project now, since that will take forever to fix all issues.

@Vurv78
Copy link
Member Author

Vurv78 commented Nov 18, 2023

Oh damn, i have much more issues, when trying to create file there is some error in e2 and hdd controller crashes:

sv: Expression 2 (HDD Controller): Runtime error 'Mismatching return types. Got n, expected void' at line 225, char 9

Guess some function doing this... and i guess there will be tons of this. Basically i guess can RIP whole project now, since that will take forever to fix all issues.

I can lift the restriction if you're referring to lambdas. Denneisk already brought this up to me but I was against it

@AlexALX
Copy link
Contributor

AlexALX commented Nov 18, 2023

After some checks, it seems like i have call function what return number but i'm calling it as void, basically like that:

@name 
@inputs 
@outputs 
@persist 
@trigger 

function number blah() {
    return 0
}

FuncName = "blah"
FuncName()

And this causes this error. before i could call any func regardless if it return or not (more flexible way) so i assume those func return check should be really applied for strict mode maybe...

@AlexALX
Copy link
Contributor

AlexALX commented Nov 19, 2023

@Denneisk i found one more issue with EGP and egpRemove func.

By some reason position shift if re-use index and parent second time after calling egpRemove.

Here is example script (partial cut from my code):

@name EGP TEST
@inputs EGP:wirelink
@outputs 
@persist LAST_I DialogData:table
@trigger 

function number draw() {
    
    local Width = 400
    local Height = 80
    
    local Col = table(vec4(0,168,168,255),vec4(255,255,255,255))
    
    local LAST_SI = DialogData[100,number]
    if (LAST_SI>0) { LAST_I = LAST_SI } else { DialogData[100,number] = LAST_I }
    
    local Parent = LAST_I
    print("ParentID: "+Parent)
    EGP:egpBox(LAST_I,vec2(256-(Width/2),256-(Height/2)),vec2(Width,Height))
    EGP:egpColor(LAST_I,Col[1,vector4]) LAST_I++
    
    EGP:egpBoxOutline(LAST_I,vec2((256+8)-(Width/2),(256+8)-(Height/2)),vec2(Width-16,Height-16))
    EGP:egpColor(LAST_I,Col[2,vector4]) LAST_I++
    
    EGP:egpBoxOutline(LAST_I,vec2((256+11)-(Width/2),(256+11)-(Height/2)),vec2(Width-22,Height-22))
    EGP:egpColor(LAST_I,Col[2,vector4]) LAST_I++

    
    
        local EntID = table()    
    DialogData[-10,table] = EntID
    
        local LAST_Y = 40
        local LeftPos = 20
        
        EntID:pushNumber(LAST_I)
        EGP:egpBox(LAST_I,vec2(LeftPos,LAST_Y),vec2(Width-40,20))
        EGP:egpParent(LAST_I,Parent)
        EGP:egpColor(LAST_I,125,125,125,255) LAST_I++

        DialogData[-5,number] = LAST_I
        DialogData[-6,number] = 40
        EntID:pushNumber(LAST_I)
        EGP:egpBox(LAST_I,vec2(0,0),vec2(1,20))
        EGP:egpParent(LAST_I,LAST_I-1)
        EGP:egpColor(LAST_I,0,0,0,255) LAST_I++
        
        DialogData[-4,number] = LAST_I
        EntID:pushNumber(LAST_I)
        EGP:egpText(LAST_I,"0%",vec2(Width/2,LAST_Y))
        EGP:egpAlign(LAST_I,1)
        EGP:egpParent(LAST_I,Parent)
        EGP:egpColor(LAST_I,200,200,200,255) LAST_I++
        
        local LAST_Y = 40
        local LeftPos = 20
        
        EntID:pushNumber(LAST_I)
        EGP:egpBox(LAST_I,vec2(LeftPos,LAST_Y),vec2(Width-40,20))
        EGP:egpParent(LAST_I,Parent)
        EGP:egpColor(LAST_I,125,125,125,255) LAST_I++

        DialogData[-5,number] = LAST_I
        DialogData[-6,number] = 40
        EntID:pushNumber(LAST_I)
        EGP:egpBox(LAST_I,vec2(0,0),vec2(1,20))
        EGP:egpParent(LAST_I,LAST_I-1)
        EGP:egpColor(LAST_I,0,0,0,255) LAST_I++
        
        DialogData[-4,number] = LAST_I
        EntID:pushNumber(LAST_I)
        EGP:egpText(LAST_I,"0%",vec2(Width/2,LAST_Y))
        EGP:egpAlign(LAST_I,1)
        EGP:egpParent(LAST_I,Parent)
        EGP:egpColor(LAST_I,200,200,200,255) LAST_I++

    return LAST_I
}

function clearLines() {
        Entities = DialogData[-10,table]
        printTable(Entities)
        for(I=1,Entities:count()) {
            EGP:egpRemove(Entities[I,number])   
        }   
}

function init() {
    DialogData = table()
    LAST_I = 1
    
    EGP:egpClear()
    EGP:egpDrawTopLeft(1)
    
    draw() 
    
    #[ Uncomment to see issue ]# 
    #clearLines()
    
    draw()    
}

if (first()) {
init()
}

If run in without "clearLines" it looks as this:
изображение

but if uncomment it - this will happend:
изображение

Did for a while to find reason of menus shifting after redraw. This doesn't happens earlier.

Also this happens if open menus and close them, after some attempts this start happening due to same bug i guess:
изображение

@Denneisk
Copy link
Member

Thanks for the report.

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

@thegrb93 regarding #2882

so you propose to remove my addon from workshop then? my project way huge to fix and recode tons of stuff because of this "it doesn't seem like a regression. Just a slightly stricter standard" ?

This change breaks some behavior when i could return only when needed and use different set of functions... now its required to not just return, but also change code with mandatory making it as expected value and not just call without needed for store output in other variable.

@thegrb93
Copy link
Contributor

@AlexALX I don't know without seeing what code of yours is broken

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

Here is super simple example

@name 
@inputs 
@outputs 
@persist 
@trigger 

function number test() {
    return 1    
}

"test"()


sv: Expression 2 (generic): Runtime error 'Mismatching return types. Got n, expected void' at line 11, char 1

Now it must be:

"test"()[number]

to work without error, which makes need to know return type of function if you even don't need anything in return from this scenario.. when you have tons of functions like that it makes issues to update all places.

if its not strict mode, why limit to this..

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

To more clarify why i need this - i have special "cmdRun" function, which can call anything, it could be function WITH return, or WITHOUT return. this feature now completely broken and not possible to call functions in this way. which broke rest of stuff i have. my propose remove this error check if chip running non in strict mode. that should solve my issues.

In other case i really can't fix this and can rip my project, because don't have way to implement this in dynamic way. (no way to call function without knowing its return type)

@thegrb93
Copy link
Contributor

thegrb93 commented Mar 28, 2024

Can you give an example of generic typing you're trying to accomplish? I've yet to see a simple example that actually proves there's an issue.

The example above is way too simple and doesn't highlight the issue.

I.e. show us why "test"()[number] is infeasible

@thegrb93
Copy link
Contributor

Going to ping @Vurv78 too since he knows more about it than I.

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

Ok lets try next scenario:

  1. i have callable string (function) which can be anything and return anything.
  2. i call this function but i dont need at this point any return (so will work as void)
  3. i can reuse same function in other places where will be needed return, and only then i'll specify its type (hardcoded).

so 2 option is broken after this changes. to fix that i need to write wrapper functions which will return void for any function which needs to be used. something like that probably...


function number test() {
    return 1    
}

function test() {
    test()[number]
}

"test"()

which makes more code and looks ugly. but yeah, probably can be "fixed" in this way... only one issue is make this for every function what could be used in such behavior...

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

oh lol
изображение
no it cannot work in this way. damn.

o kthen i'm give up. guess time to rip project.

@thegrb93
Copy link
Contributor

You're still not showing why you need it to be "test"(), are you trying to mix functions of different return types? I don't think E2 is fundamentally supposed to be able to do that.

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

You're still not showing why you need it to be "test"(), are you trying to mix functions of different return types? I don't think E2 is fundamentally supposed to be able to do that.

"test" will be in variable, which will be in another place. then wrapper which can run any function without knowing its return type and don't need return in response. Its hard to explain why i need this, but all my project is based on callable strings and working with functions in DYNAMIC way

In which revision this was added? guess only one option left for me just give link to wire at github with those rev with instruction how install wire from github and not workshop version.

@thegrb93
Copy link
Contributor

Why not make all the functions it's capable of accessing have the same return type?

@thegrb93
Copy link
Contributor

I guess that's the solution you're trying to avoid.

I don't remember when the compiler update was done. Sometime last year.

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

Why not make all the functions it's capable of accessing have the same return type?

yes i also possible, but then in way like "function_name_return_number" and wrap as in example above but with different name, and then update all places which used it. as i said i can use function with hardcoded return AND in some places without need to return anything (so 2 usages for different purposes).

and issue maybe not about design of it, but fact that in earlier versions of wiremod it did WORK without needing anything like that! then you made incompatible changes and now need to review tons of code and fix stuff what did WORK before without issues.

Why not call then expression v3 or something? to not make incompatible changes.

besides this change, also egp change broke all menus etc, which also need update ALL code i have. So for me really easier to write instructions how install older wiremod than try to fix my addon and spend WEEKS on that.

@thegrb93
Copy link
Contributor

string function calls had to do type checking at runtime before which would crash the chip if type mismatching was detected. Now those issues are prevented at compile time which improves performance as well.

@thegrb93
Copy link
Contributor

The tradeoff is you can't have a generic string call that can call any function type

@AlexALX
Copy link
Contributor

AlexALX commented Mar 28, 2024

Ok, i just did put link to older wiremod version, and said that my addon cannot work with newer version of wire from workshop. Thats all what i can do it seems. Having no time to review & update code. Thanks anyone who tried to help.

@Vurv78
Copy link
Member Author

Vurv78 commented Mar 28, 2024

Here is super simple example

@name 
@inputs 
@outputs 
@persist 
@trigger 

function number test() {
    return 1    
}

"test"()

sv: Expression 2 (generic): Runtime error 'Mismatching return types. Got n, expected void' at line 11, char 1

Now it must be:

"test"()[number]

to work without error, which makes need to know return type of function if you even don't need anything in return from this scenario.. when you have tons of functions like that it makes issues to update all places.

if its not strict mode, why limit to this..

I don't see a problem with allowing this for statements, actually.

Checking the code, I think it was an oversight, at least for string calls, that this doesn't work.

if r ~= ret_type and not (ret_type == nil and r == "") then

The other cases for variadic functions and builtins do allow not specifying the return,

But it's not checked here:

Someone can pr to fix that, pretty simple fix. I only had a problem with this for lambdas, where it would incur a perf hit to allow that (but re-thinking, you could just duplicate the code with used_as_stmt without the check..)

@thegrb93
Copy link
Contributor

Someone can pr to fix that, pretty simple fix

Just do it real quick dude. A quick unit test would be good too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants