KeenSoftwareHouse / SpaceEngineers

2.93k stars 896 forks source link

Get Ore Markers in the Programmable Block #567

Open DoubleCouponDay opened 7 years ago

DoubleCouponDay commented 7 years ago

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

DoubleCouponDay commented 7 years ago

Its not quite ready for the public.

spacebuilder2020 commented 7 years ago

Does this fix the memory leak on planets when a over-sized sensor range (500) is used.

Anyway, great work.

DoubleCouponDay commented 7 years ago

No bug fix but removes the need to make that kind of lag :-)

spacebuilder2020 commented 7 years ago

Nice, well now we just wait. :p

kleshchevand commented 7 years ago

I'm not sure, but isn't this too specific? Detecting ore is good and all, but why limit code with ore? If you are giving access to HUD markers shouldn't it be all markers? Like, user specifies in script/function what block he wants to get markers from - if user passes ore scanner block to script/function, it will return markers from said scanner, if user passes antennae it will get all marks accessible to this specific antennae, non-broadcasting block - marker of said block if present ... Examples of script that can use this functionality: "autiopilot" can use visible beacon/antennae markers to reach target or follow specific route, keep distance from ships in fleet. Another script can mark some damaged blocks (if they can be marked) to make drones approach and repair blocks. Potential is immense... Besides keeping code universal (even if only ore will be exposed to API) is a good practice.

DoubleCouponDay commented 7 years ago

@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...

kleshchevand commented 7 years ago

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? public interface IMyOreDetector : IMyFunctionalBlock, IMyMarkerProvider

P.S. Sorry if my English is a bit rough. Also I'm surprised how decentralized whole marker system is...

DoubleCouponDay commented 7 years ago

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 https://github.com/KeenSoftwareHouse/SpaceEngineers/blob/e2b5b14e1e798b7e4579b1ee38a1a5a89ae556f0/Sources/Sandbox.Game/Game/GUI/MyHud.cs https://github.com/KeenSoftwareHouse/SpaceEngineers/blob/0807af9557057a76f8724e6370cb3f440d8c0979/Sources/Sandbox.Game/Game/Screens/MyGuiScreenHudSpace.cs

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.

rexxar-tc commented 7 years ago

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.

DoubleCouponDay commented 7 years ago

Hmm ill revise it and get back to you.

DoubleCouponDay commented 7 years ago

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.