Get Ore Markers in the Programmable Block #567
Conversation
|
Its not quite ready for the public. |
Major Change: GetOreMarkers() can now get updates without needing a radio antenna.
|
Does this fix the memory leak on planets when a over-sized sensor range (500) is used. Anyway, great work. |
|
No bug fix but removes the need to make that kind of lag :-) |
|
Nice, well now we just wait. :p |
|
I'm not sure, but isn't this too specific? |
|
@kleshchevand I agree on the usefulness of creating api hooks for all types of hud markers. I think its a glimpse of future ingame scripting. However, the hooks must be associated with a block ingame and this is the ore detector's one :]. Also i think a highly specific pull request has a higher chance of being accepted... maybe... |
|
I can see that all markers are entirely separate classes which is odd and prevents implementation of more universal system, but may be it still can benefit from something like this? P.S. Sorry if my English is a bit rough. Also I'm surprised how decentralized whole marker system is... |
|
An interface would only make the code readable so we would know which blocks need marker methods in their component class. Theyve kindof already done this in a static way (ew whats that smell?). I am not brave enough to refactor that for them :D I was hoping MyHud would be cleaner than it is but it's public ore List is not filtered at all; not even each seperate deposit is refined into one marker. They have decided to filter only under specific conditions: antenna on and owned, player present and friendly. The way I got this working was to not touch the HUD code yet copy its filtering method into MyOreDetector. Refactoring that would mean filtering every time an ore detector is switched on. Is that a good thing or a bad thing and which has the biggest performance cost? filtering twice under very specific conditions or filtering once under general conditions? Think in terms of why they would design the marker system so decentralized. In their view, ore markers have a single purpose: to display on the hud. Therefore lets split that code in half so its only used when needed. Actually we could just leave the procession as is but make a new method which MyGuiScreenHudSpace and MyOreDetector have in common. I cant think of a class structure where that idea would mesh well but the more types of markers that need the same kind of filtering, the better. |
|
Oh man, I literally just finished an identical feature. There's a very serious problem with your implementation, though. There's no rate limiting, so the PB can spam the ore detector a thousand times every tick and ruin simspeed. Also, your changes to MyOreDetectorComponent weren't necessary, you can just pass Update(pos, false) and it will update fine. |
|
Hmm ill revise it and get back to you. |
… variable but still needs to be tested.
…s its previous version.
|
I see what you mean about the duplicate variable. That method was a real headache to debug so I just wrapped all states I don't want to break into a single if block. I can do the same thing with bool checkControl no problem. Thing is some changes are necessary to MyOreDetectorComponent in order to call for markers without presence of a player or antenna. However, if you guys are intending the opposite (for future ore detector API changes) then ill try to tailor it to your specifications. |
| { | ||
| ticksSinceOreMarkersCalled++; | ||
| MethodCallRateCounter++; |
There was a problem hiding this comment.
wouldn't ++MethodCallRateCounter be better?
There was a problem hiding this comment.
Increment before viewing the number? why does it matter?
There was a problem hiding this comment.
Because than no duplicate of the variable is created which would be returned at this point.
Like this:
varType incrementBeforeReturn() { var = var +1; return var; }
varType incrementAfterReturn() { varType prev = var; var = var +1; return prev; }
incrementing before before return therefore reduces memory consumption and runtime
There was a problem hiding this comment.
The compiler does this automaticly, no need to worry
List is a reference type anyway, the `ref` qualifier is useless unless you want to assign to the argument, which defeats the point of making it an argument and not a return value.
List is a reference type anyway, the `ref` qualifier is useless unless you want to assign to the argument, which defeats the point of making it an argument and not a return value.
Removed unnecessary ref qualifiers.
Adds a new method to IMyOreDetector which allows players to get, in code, the ores they see on their HUD. This revolutionizes the automated mining process.
It fills in the player's list with structs. Each struct has an ore element name and a Vector3D. Im sure people will appreciate this intuitive solution by mining straight to the deposit as opposed to clearing everything. Previous asteroid-detecting-sensor designs have now been made clunky, laggy, obsolete.
Ive tweaked the voxel updating method to get ore markers even when:
This is because a player should not have to be present for a drone to go about its tasks.
If you want to test this pull request, here's a programmable block script to get you up and running:
https://github.com/DoubleCouponDay/Ingame-Scipting-Collection/blob/master/SEScripting%20Collection/SEScripting%20Collection/OreDetectorSourceTest.cs