wiremod / wire-extras

Community Contributed Wire Extras Repository (formerly UWSVN). These are addons which may be of use to wiremodders but do not undergo the same level of critique that the official repos do.
Apache License 2.0
83 stars 67 forks source link

Sychronize stcontrol and ftrace #102

Closed dvdvideo1234 closed 3 years ago

TomyLobo commented 3 years ago

Are you aware that you're renaming functions that have existed for more than a year?

dvdvideo1234 commented 3 years ago

Yes, but it is needed since the user can utilize all three types of filters. Between single entity unit ( ENU ), entity array ( EAR ) and function callback ( FNC ) the first one is the fastest and least customizable and the last is the slowest and most customizable. However the first two modes is what the source engine utilizes natively, so to me, this is a decent change. My current addon has these functions and the Wiki is based on my dedicated addon repo, not the wire extras, so yeah.

dvdvideo1234 commented 3 years ago

The last commit does not change any E2 APIs but distinguishes then from local function names. Users can now populate all three filters ans switch them on the fly... Or remove the hit filtering all together. Modification of the three filter types is also available. My addon should be able to work with native entity arrays as they are implemented in the engine are quite fast. Here is what the wiki says about using functions as filters.

dvdvideo1234 commented 3 years ago

@thegrb93 @TomyLobo

How do you feel about renaming the abbreviations as follows:

  1. Ear becomes Array obviously. This seems like the most logical name to be.
  2. Enu becomes Unit obviously. Short to write and different than Entity as I already have getEntity that retrieves the trace.entity and must not overlap with with the future getEntity retrieving the entity filter of Enu
  3. Fnc well... I think we have several option here since function is quite long as well. I am thinking of:
    • Routine as Sequence of actions regularly followed
    • Action as Process of doing something, typically to achieve an aim

I can prefix or suffix these with Filter to tell the user this is intended to to be used for the trace.filter.

For DispFlags I think of keeping the trace result name as this is the convention the other functions are currently using.

What do you think of this idea ?

I do not want to change anything until I get your approval as well since I am not a native English speaker. :+1:

thegrb93 commented 3 years ago

I think those suggestions look fine. Function can be Func, most people will know that abbreviation.

dvdvideo1234 commented 3 years ago

Acton seems shorter though 😀 thing i am gona use that instead ☺

TomyLobo commented 3 years ago

Well you already know my opinion. These functions have existed for more than a year, so we should not rename them.

At the very least keep the old names as hidden aliases. I don't remember how that works, though. Ask on #dev-wiremod.

EDIT: To clarify, aliases are easy: https://github.com/wiremod/wire/blob/master/lua/entities/gmod_wire_expression2/core/find.lua#L487 I meant the hiding part.

dvdvideo1234 commented 3 years ago

Correct. I will leave backwards compatibility hidden functions, but if I do, I must set the filter to use a function as default option.

Edit: Or... Wait better not. Will just make aliases for the old functions manipulating the function filter and it will be enough along with some others I noticed

e2function ftrace ftrace:addEntHitSkip(entity vE)
e2function ftrace ftrace:remEntHitSkip(entity vE)
e2function ftrace ftrace:remEntHitSkip()
e2function ftrace ftrace:addEntHitOnly(entity vE)
e2function ftrace ftrace:remEntHitOnly(entity vE)
e2function ftrace ftrace:remEntHitOnly()
e2function ftrace ftrace:remEntHit()
e2function ftrace ftrace:remHit()
e2function ftrace ftrace:remHit(string sM)
e2function ftrace ftrace:addHitSkip(string sM, number vN)
e2function ftrace ftrace:remHitSkip(string sM, number vN)
e2function ftrace ftrace:addHitOnly(string sM, number vN)
e2function ftrace ftrace:remHitOnly(string sM, number vN)
e2function ftrace ftrace:addHitSkip(string sM, string vS)
e2function ftrace ftrace:remHitSkip(string sM, string vS)
e2function ftrace ftrace:addHitOnly(string sM, string vS)
e2function ftrace ftrace:remHitOnly(string sM, string vS)
dvdvideo1234 commented 3 years ago

@TomyLobo

All aliases are generated automatically via Alias:.... It is not hard to change blahHit to blahAction but I am OK with that.

The new wiki with updated backwards compatibility: https://github.com/dvdvideo1234/ControlSystemsE2/wiki/FTrace

TomyLobo commented 3 years ago

I was talking about hiding the deprecated versions from e2helper completely, but @Divran tells me there is no way to do that :/

thegrb93 commented 3 years ago

I was talking about hiding the deprecated versions from e2helper completely, but @Divran tells me there is no way to do that :/

Don't you just need to remove them from the e2descriptions.lua

dvdvideo1234 commented 3 years ago

@TomyLobo @thegrb93 Ah. I think this is not realted to my addon then 🤔 I do not have any changes on this file. If you insist I will remove the documentation for the old versons of the functions and leave only the new API docks. Are you OK with this.

Can I just remove the description for the deprecated functions ?

dvdvideo1234 commented 3 years ago

@thegrb93 Should I remove the description for the alias finctions??

thegrb93 commented 3 years ago

I think aliases should not be in the helper, yeah. I'll have time to look over the code this week.

thegrb93 commented 3 years ago

The rest looks ok. I can merge once you fix your loops and I verify there's no lua errors.

thegrb93 commented 3 years ago

I'll test it tonight

dvdvideo1234 commented 3 years ago

@thegrb93

Please do not merge right away I have a bit more changes to add. Renamed dumpItem do keep the naming convention. Anyways, do these two appear different to E2 with same parameters and name. I assume they are separate, but they utilize the same description:

__e2setcost(3)
e2function array stcontrol:getPower()
  if(not this) then return {0,0,0} end
  return {this.mpP, this.mpI, this.mpD}
end

__e2setcost(3)
e2function vector stcontrol:getPower()
  if(not this) then return {0,0,0} end
  return {this.mpP, this.mpI, this.mpD}
end