Welcome to Keen Software House Forums! Log in or Sign up to interact with the KSH community.
  1. You are currently browsing our forum as a guest. Create your own forum account to access all forum functionality.

ModAPI Critical issues

Discussion in 'Modding API' started by rexxar, Dec 24, 2016.

Thread Status:
Not open for further replies.
This last post in this thread was made more than 31 days old.
  1. rexxar Senior Engineer

    Messages:
    1,530
    I've been assigned the task of cleaning up all critical issues in ModAPI. If you've got a problem, post it here.

    I'm defining critical issue as something that crashes the game or is just plain broken. I am not taking feature requests right now. If you post a feature request, I'm just gonna delete it. (This includes missing functionality)

    edit: This also includes PB API. I have a few fixes pending (like adding a using for Linq), but no new features.
     
    Last edited: Dec 24, 2016
    • Like Like x 1
  2. Gwindalmir Senior Engineer

    Messages:
    1,004
    Adding blocks to a grid in a synced manner. There are methods already, but they don't work right.
    To add blocks, I have to create a new grid, and merge them.
     
  3. Draygo Senior Engineer

    Messages:
    1,297
    Crash report:

    Responsible code:
    Code:
    MyTransparentGeometry.AddPointBillboard("WhiteDot", Color.White.ToVector4(), dotpos, 0.0008f, 0f, 0);
    Crash:
    Code:
    2016-12-23 23:30:41.203 - Thread:  1 ->  Exception occured: System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary.
      at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
      at VRage.Game.MyTransparentGeometry.CreateBillboard(MyBillboard billboard, MyQuadD& quad, String material, Vector4& color, Vector3D& origin, Vector2 uvOffset, Int32 customViewProjection, Single reflectivity)
      at VRage.Game.MyTransparentGeometry.CreateBillboard(MyBillboard billboard, MyQuadD& quad, String material, Vector4& color, Vector3D& origin, Int32 customViewProjection, Single reflection)
      at VRage.Game.MyTransparentGeometry.AddPointBillboard(String material, Vector4 color, Vector3D origin, Int32 renderObjectID, MatrixD& worldToLocal, Single radius, Single angle, Int32 customViewProjection, BlenType blendType)
      at DockingAssist.DockCore.DrawDot(Vector2D Origin, Color _color)
      at DockingAssist.DockCore.DrawCenterDot()
      at DockingAssist.ConnectorAssist.Draw()
      at DockingAssist.DockCore.Update()
      at Sandbox.Game.World.MySession.Draw()
      at Sandbox.Game.Gui.MyGuiScreenGamePlay.Draw()
      at Sandbox.Graphics.GUI.MyScreenManager.Draw()
      at Sandbox.Graphics.GUI.MyDX9Gui.Draw()
      at Sandbox.MySandboxGame.PrepareForDraw()
      at Sandbox.Engine.Platform.Game.UpdateInternal()
      at Sandbox.Engine.Platform.Game.RunSingleFrame()
      at Sandbox.Engine.Platform.FixedLoop.<>c__DisplayClass1.<Run>b__0()
      at Sandbox.Engine.Platform.GenericLoop.Run(VoidAction tickCallback)
      at Sandbox.Engine.Platform.Game.RunLoop()
      at Sandbox.MySandboxGame.Run(Boolean customRenderLoop, Action disposeSplashScreen)
      at SpaceEngineers.MyProgram.Main(String[] args)
    --- Automerge ---
    Looking into it further, this is because the signature of AddPointBillboard changed, and the priority argument was removed, sliding the 0 over to the customviewprojection, which passing 0 crashes the game.
     
  4. Phoera Senior Engineer

    Messages:
    1,713
    my current issue is that there is no some methods in MyTransparentGeometry which still accepts Vector3, but not Vector3D, like AddTriangleBillboard
     
  5. Malware Master Engineer

    Messages:
    9,631
    Crap; made a post about missing functionality, reconsidered because you said "no new features", then reread your post just to see you explicitly state "missing functionality"... :p

    Here I go again then; for programmable block of course:

    • Landing gear "is in proximity" property
    • Thruster effective max thrust, taking into consideration variant efficiency (ions, atmospherics). Can't just be a fix of the current max thrust because that tells us what the engine is capable of doing. Should be its own property. Didn't someone make a GitHub PR about this?
    • Proper Jump Drive properties, no more DetailInfo parsing
    • Jump Drive engage (yeah, yeah, I know... I just really want it :p )
    • Separate DisplayText fields for antennae and beacons. It might not seem like a big issue, but it is a serious PITA to have to keep track of the current names of these for reaquire. Names should be possible to make unique and unchanging. Please? If these aren't set, fall back to the CustomName - this way older builds aren't affected.
    • Select font on text panels
     
    Last edited: Dec 24, 2016
    • Agree Agree x 2
  6. Whiplash141 Junior Engineer

    Messages:
    958
    Is this the correct place to mention that there isn't a way via in-game programming to set a value stored in a combo box?

    Like to set the font type of a text panel, the variable input must be of type MyTextPanel :/
     
  7. Malware Master Engineer

    Messages:
    9,631
    Yeah, the terminal property is set up wrong. I asked for a proper way to set that font though. I really despise having to use (ugh) actions and terminal properties, but replacing all those with "real" properties and methods is asking too much...
     
  8. Cheetah97 Trainee Engineer

    Messages:
    31
    Sometimes, it is possible to assign the same Entity.Name for two different entities. This may confuse MyVisualScriptLogicProvider.

    P.S. It would be great if MVSLP functions were also accessible with EntityID :p
     
  9. Phoera Senior Engineer

    Messages:
    1,713
    not possible, Name must be unique.
     
  10. Wicorel Senior Engineer

    Messages:
    1,243
    Wico's list of Missing/Broken In-Game API


    • effective mass of grid (as used by physics)

    • effective thrust from thruster (in the current environment)

    • isReadyToLock() for landing gears

    • ConnectedGrid for rotors and pistons. So we can find out 'local' grids. Connectors already have this

    • getting/Setting font on LCDs (IMO this could maybe be just MonospaceON/OFF) (it's a list in UI)

    • getting/setting direction and mode for autopilot in RC (it's a list in UI)

    • IsPressurizationEnabled for airVents so we don't have to parse the DetailedInfo.

    • Get grid CenterOfMass

    • Get/Set grid name (and get on sub-grids)

    • Control over wheels for propulsion (probably a feature, I'm afraid)

    • Need to verify power info works, especially in DS

    • Add: Need CanScan(Vector3D targetPos)
      and
      CanScan(double distance, Vector3D targetDirection)
    • Add2:: When a player is using a remote control, they are detected by sensor as 'animal' not 'player'. I did not test with camera raycast, but it may be the same.
     
    Last edited: Dec 26, 2016
    • Agree Agree x 2
  11. gothosan Junior Engineer

    Messages:
    723
    As @Malware said, selecting LCD font is broken, throw exception and so does combobox for remote control block do the same.
     
  12. Malware Master Engineer

    Messages:
    9,631
    Oh - I forgot. The super-annoying and troublesome autorenaming of the thrusters to append direction. Get that away from the customname!!! My thruster is not named "Thruster (Up) (Up) (Up) (Up)". It's named "Thruster". Hmpf.
     
    • Agree Agree x 3
    • Funny Funny x 1
  13. funkrusha Trainee Engineer

    Messages:
    5
    something is broken with shipmass
    ShipController.CalculateShipMass() returns weird values

    the correct value in the image is that one, that i assume based on current thrust

    see attached image for more details[​IMG]
     
  14. d4rky1989 Apprentice Engineer

    Messages:
    332
    Example 1 (broken/bug?):
    Official documentation of the method "MySessionComponentBase.IsRequiredByGame":
    /// <summary>
    /// Indicates whether a session component should be used in current configuration.
    /// Example: MyDestructionData component returns true only when game uses Havok Destruction
    /// </summary>
    It seems IsRequiredByGame is intendet to determine whether a machine should load a component. So I want to use it to get certain components loaded only on the server.
    Code:
    public override bool IsRequiredByGame {
        get {
            return MyAPIGateway.Multiplayer.IsServer;
        }
    }
    Unfortunatley it does not work. The component will never get loaded. It seems IsServer always returns false. I guess the object that is returned by "MyAPIGateway.Multiplayer" is not yet completely initialized when IsRequiredByGame gets invoked.
    A mod should only be loaded by the game after all other components that are accessible by it (like everything inside MyAPIGateway) are properly loaded.

    Example 2 (missing feature):
    It is pretty awkward and not intuitive to get access to some basic game information. For instance factions. There is no simple way to get a list of all factions. I'm even wondering why the type returned by MyAPIGateway.Session.Faction is called IMyFactionCollection. It is a fact that the interface IMyFactionCollection does not extend the ICollection interface.
    The only way to get a list/set of all factions I know is:
    Code:
    readonly HashSet<IMyFaction> _tmp_factions = new HashSet<IMyFaction> ();
    readonly List<IMyIdentity> _tmp_identities = new List<IMyIdentity> ();
    HashSet<IMyFaction> GetAllFactions ()
    {
        _tmp_factions.Clear ();
        _tmp_identities.Clear ();
        MyAPIGateway.Players.GetAllIdentites (_tmp_identities);
        foreach (var identity in _tmp_identities)
        {
            var playerFaction = MyAPIGateway.Session.Factions.TryGetPlayerFaction (identity.PlayerId);
            if (playerFaction != null)
                _tmp_factions.Add (playerFaction);
        }
        return _tmp_factions;
    }
    This is pretty everything but intuitive.


    Example 3 (please make the API design less tightly coupled to the game engine design):
    Faction IDs, player IDs, identity IDs... IDs here, IDs there IDs everywhere :p
    The current API design forces a modder to cope with this badly ID-based design. To be honest I don't even know the exact difference between the player ID and the identity ID which are both somehow associated with a player.
    I'm sure the IDs are important because of the multiplayer part of the game. Nevertheless they are not needed by the modders if the API had no ID based design. The IDs could be embedded in faction, player and identity types. Whenever the API needs the ID beneath the engine hood (for instance when sending a message to a certain machine) it can take it from the object itself.
    I'd like to have a more OO design like
    Code:
    void JoinFactionOfPlayer (IMyPlayer playerWithFaction, IMyPlayer playerToJoinFaction)
    {
        if (playerWithFaction.GetFaction () != null)
            playerWithFaction.GetFaction().AddPlayer (playerToJoinFaction);
    }
    instead of an ID based design
    Code:
    void JoinFactionOfPlayer (IMyPlayer playerWithFaction, IMyPlayer playerToJoinFaction)
    {
        var faction = MyAPIGateway.Session.Factions.TryGetPlayerFaction (playerWithFaction.PlayerId);
        if (faction != null)
            MyAPIGateway.Session.Factions.SendJoinRequest (faction.FactionId, playerToJoinFaction.PlayerId);
    }
    Edit:
    Well forget about that 2nd part. Just discovered that it is possible using the object builder
     
    Last edited: Dec 28, 2016
  15. Whiplash141 Junior Engineer

    Messages:
    958
    Oh found another annoying bug. IMyLargeTurretBase.IsShooting boolean is broken. It only returns true when you toggle shooting On/Off through terminal commands. It does not ever return true when the turret is being fired via manual control or AI control.

    GIF:


    Bug Report Link
     
    • Agree Agree x 1
  16. rexxar Senior Engineer

    Messages:
    1,530
    Done.
    Done.
    Not happening.
    Out of scope :p
    Done.
    You do understand you just asked me to rewrite the entire game, right? There is nothing at all wrong with EntityID and IdentityID. You must have some way of looking up objects you don't already have a reference to.

    Actually d4ky, after further consideration, your first point also requires too much change. Your second point has been fixed, I added a Factions property to IMyFactionCollection. Getting the objectbuilder is a terrible idea in almost all circumstances. Avoid it at all costs.
     
    Last edited: Dec 29, 2016
  17. Phoera Senior Engineer

    Messages:
    1,713
    but what about making VisSc methods avaliable without entity name?

    also, give me method to read player toolbar, so my GtD mod can remove only such entries(also i don't remember is there method to clear one entry?) =P


    PS: nice work Rex!
     
  18. rexxar Senior Engineer

    Messages:
    1,530
    I can't touch visual scripting.
     
  19. Phoera Senior Engineer

    Messages:
    1,713
    may be add analogs into MyAPIGateway?
     
  20. Malware Master Engineer

    Messages:
    9,631
    I will of course defer to your judgement on that, but if I may attempt to defend it: I do consider this broken for the API. The API gives the impression that the CustomName is supposed to be predictable. For instance, it is impossible to use GetBlockWithName to retrieve thrusters, because their CustomName depends on whether or not there's a pilot in a seat somewhere and as such constantly changes. While antennae and beacons are still PITAs, at least I can track those. That's not possible for thrusters, because their names are unpredictable. Not only that, but it is just too easy for this to just accumulate. I haven't been able to track down the ways to get names like I suggested above with (Up) (Up) (Up) etc.

    It is true that there are workarounds, but obviously these will require more instructions to run. Also one cannot really expect beginners to grasp this concept, making it more difficult than necessary to deal with thrusters because they break the convention of all the other blocks.
     
    • Agree Agree x 1
  21. rexxar Senior Engineer

    Messages:
    1,530
    @Malware Ah, when you put it that way, it is a problem. It's still out of scope for this project, but I'll put in a ticket for it.
     
  22. Malware Master Engineer

    Messages:
    9,631
    @rexxar If you say it's out of scope, it's out of scope and I won't nag any further about it :) Thanks for ticketing it though.
     
  23. Wgooch Trainee Engineer

    Messages:
    2
    There are two issues with the PB API that I've run into recently.

    The "CurrentMethodCallCount" property in within "MyGridProgram.Runtime" never actually reports the current call count. See the example below to reproduce this issue.
    Code:
    public void Main()
    {
       int x = 0;
       for(int i = 0; i < 100; i++)
       {
         x = TestMethod(x);
       }
       // Always prints "x is 100"
       Echo("x is " + x);
    
       // Always prints "Runtime.CurrentMethodCallCount is 0"
       Echo("Runtime.CurrentMethodCallCount is " + Runtime.CurrentMethodCallCount);
    }
    
    // Dummy method purely to increment the call count
    int TestMethod(int x)
    {
       return x + 1;
    }
    
    Looking at the most recent source code, it looks like "CurrentMethodCallCount" is never actually set. However, as evidenced by the next issue, there is a variable responsible for recording the method call count(see edit), it just isn't being reported.

    Edit: I looked over the source code further, and I found that method calls aren't even counted as they were previously. A method named "CountMethodCalls" appears to be the only instance where the current number of calls is changed, but it does not appear to be injected into any code in practice. Only call depth is currently kept track of, but there is still no way to access the current depth. Perhaps CurrentMethodCallCount should be marked as obsolete and a new property introduced?


    Iterator methods are not properly decrementing the method call count when they exit through a yield return statement. See the example below to reproduce this issue.
    Code:
    public void Main()
    {
       for(int i = 0; i < 100; i++)
       {
         var iter = IteratorMethod();
         iter.MoveNext();
       }
    }
    
    IEnumerator<int> IteratorMethod()
    {
       while(true)
       {
         yield return 1;
       }
    }
    
    Running the above code a few times will effectively stop all PBs from executing any further code as the call count (which is shared between all blocks and never reset) will increase until it reaches the limit. This effect persists until the game is rebooted.

    I've debugged this issue a bit and outlined my findings below. Hopefully they'll be of some use.

    Looking at the final output of a compiled script, the engine has wrapped all method bodies in a try finally block pair with a call to "VRage.Compiler.IlInjector.EnterMethod()" preceding the block pair, and a call to "VRage.Compiler.IlInjector.ExitMethod()" within the finally block. This effectively tracks call depth under traditional circumstances, but iterator methods fail here. More specifically, the finally block will not be called if the user exits the method by use of a yield return statement, and in fact, the only ways I've been able to get the finally block to execute are the following:
    Exit the method using a yield break statement, call Dispose() on the iterator itself, allow the method to run to the end of the block, or trigger an exception while the method is still on the stack.

    Detecting iterator methods and return yield statements is rather easy with Roslyn, so one solution to this issue may be to inject a "VRage.Compiler.IlInjector.ExitMethod()" call preceding all yield return statements and a "VRage.Compiler.IlInjector.EnterMethod()" proceeding all yield return statements. Regardless of how this is solved, I would certainly appreciate it if the solution was more than merely making the use of iterator methods illegal :p
     
    Last edited: Dec 31, 2016
    • Informative Informative x 1
  24. gothosan Junior Engineer

    Messages:
    723
    Umm.. sorry but woulden't that always print "x is 100" since the Echo command comes after the loop?
     
  25. Wgooch Trainee Engineer

    Messages:
    2
    If I understand what you're asking, then yes, that is the intent. What is meant to be demonstrated is that x will increase in value as the call to TestMethod is made, but CurrentMethodCallCount does not. If CurrentMethodCallCount was working as it previously did, it would be expected to have a value equal to x or x + 1 if we are counting the call to Main. If we run the code however, we find that CurrentMethodCallCount is never actually anything other than 0.

    Your post prompted me to look over the source code again, and I've edited my original post with some further information if you're interested.
     
    Last edited: Dec 31, 2016
  26. Wicorel Senior Engineer

    Messages:
    1,243
    I have one more

    • The time function that is used for TimeStamp in MyDetectedEntityInfo is not available to in-game scripts. (It's name is something like TimeGameRunningInMilliseconds) This means that the times cannot be compared to 'now' and 'old' saved detections removed from a list.
     
    • Agree Agree x 1
  27. rlb Trainee Engineer

    Messages:
    7
    Link to API bug report
    This is a more specific thread, maybe I should have put it here first.
    In short, conveyor sorter API method RemoveItem is not working properly. AddItem and RemoveItem appear to be the same code, line for line, at least on Github.
     
  28. Duckroll Trainee Engineer

    Messages:
    73
  29. Malware Master Engineer

    Messages:
    9,631
    The method counter is indeed obsolete and not in use. You don't need to worry about that number at all. As for the iterator issue it has been known for some time and a solution is in the works.
     
    • Like Like x 1
  30. rexxar Senior Engineer

    Messages:
    1,530
    Fixed
    Not fixing, but there's a workaround in place.
    Fixed
    Not fixing. This is intentional.
     
    • Like Like x 1
Thread Status:
Not open for further replies.
This last post in this thread was made more than 31 days old.