MVP of grid furniture examine action #1613

AngelicosPhosphoros · 2022-06-11T02:54:58Z

Summary

SUMMARY: Features "Allow use grid powered welders and soldering irons by [e]xamining"

Purpose of change

#1612
Grid powered furniture should be usable by examining it or it would be worse than vehicle powered ones.

Describe the solution

Added new examine action use_furn_fake_item which allows players to activate crafting pseudo items from furniture.
Added use_furn_fake_item for grid welder and grid soldering iron.
Copied usages from soldering iron and welder to their fake counterparts.
Rewritten and generalized vehicle welding usage hack to use furniture too.
Little changes in tests.
Added TODOs for further improvements.

Describe alternatives you've considered

Creating separated iexamine actions for each furniture type.
Creating some hidden persistent fake items to avoid rewriting repair hack.

Testing

One time usages tested by placing soldering iron, spawning nurse, getting bleeding and cauterizing.
Repair activity changes tested using:
- Repair items using spawned welding cart
- Try to repair items using spawned welding cart without charges
- Repair items using grid soldering iron and grid welder
- Try to repair items using grid soldering iron without power in grid
- Repair items using soldering iron and tailor kit in inventory
- Repair items using soldering iron and tailor kit standing in nearby tile
Watched variable changes during all manual testing using debugger.

Additional context

See #1612

AngelicosPhosphoros · 2022-06-11T03:44:35Z

Coolthulhu · 2022-06-11T13:23:31Z

src/activity_handlers.cpp

+//
+// This relies on fact that repairing with real tools
+// never use `player_activity::coords`
+// and use `player::activity::values` with only one item.


Check out activity_actor_definitions.h for something better than the old, ugly coords/values vector hackery.

Yes, this is the plan.
I would do that in further work on that issue.
#1612

I just want to split work into chunks to make it easier.

Coolthulhu · 2022-06-11T13:30:04Z

src/activity_handlers.cpp

+            }
+        }
+
+        static void return_unused_charges( hack_type_t hack_type, item &tool, const tripoint &position ) {


Discharge/recharge "cycle" can change the way energy is distributed.
Would be better to note how much energy was in the grid at start, then consume the difference later.

I reimplemented original algorithm. Probably, your version is better and more consistent with one time usages on examine action.

Would do it little later in that PR.

AngelicosPhosphoros · 2022-06-13T20:38:07Z

@Coolthulhu This PR is finished and works stable when I playing.
I think, we can merge it, than I would open another PR with further improvements.

Coolthulhu · 2022-06-14T15:08:37Z

src/activity_handlers.cpp

+    const vehicle &veh,
+    int interact_part_idx )
+{
+    if( activity.id() != ACT_REPAIR_ITEM ) {


Does it happen? Is it expected to happen?
Needs a comment documenting when is this desirable or a debugmsg describing that an undesirable and unexpected event happened.

It doesn't happen but allowed.

I moved this code from vehicle.cpp to make hack code more encapsulated in one place so it would be easier to remove later.

Coolthulhu · 2022-06-14T15:10:02Z

src/activity_handlers.cpp

+        raii_fake_tool( const raii_fake_tool & ) = delete;
+        raii_fake_tool( raii_fake_tool && ) = delete;
+        raii_fake_tool( const player_activity &activity ): hack_type( get_hack_type( activity ) ) {
+            if( !hack_type ) {


Expected or unexpected? Needs documentation or debugmsg.

Was needed to make RAII easier. Removed now.

Coolthulhu · 2022-06-14T15:12:36Z

src/activity_handlers.cpp

+        item &get_fake_tool() {
+            return fake_tool;
+        }
+        ~raii_fake_tool() {


RAII sounds unusual and fragile here. Resource consumption should be made explicit, even if it makes it more verbose.
RAII can print debugmsgs on skipped discharge or discharge being called twice.

Well, old code used RAII here, I just didn't want to change it much.

Removed RAII.

Implementation parts: * Added new examine action `use_furn_fake_item` which allows players to activate crafting pseudo items from furniture. * Added `use_furn_fake_item` for grid welder and grid soldering iron. * Copied usages from soldering iron and welder to their fake counterparts. * Rewritten and generalized vehicle welding usage hack to use furniture too. * Little changes in tests. * Added TODOs for further improvements. Testings: * One time usages tested by placing soldering iron, spawning nurse, getting bleeding and cauterizing. * Repair activity changes tested using: - Repair items using spawned welding cart - Try to repair items using spawned welding cart without charges - Repair items using grid soldering iron and grid welder - Try to repair items using grid soldering iron without power in grid - Repair items using soldering iron and tailor kit in inventory - Repair items using soldering iron and tailor kit standing in nearby tile * Watched variable changes during all manual testing using debugger. Additional context: See cataclysmbnteam#1612

AngelicosPhosphoros · 2022-06-15T19:59:20Z

@Coolthulhu Removed usage of RAII and added comments where requested.

Coolthulhu · 2022-06-21T20:21:33Z

Alright, looks good on paper and I didn't manage to get it to break in manual testing.
It will probably bug out in some edge cases like all big changes inevitably do, so stay tuned for player feedback.
One thing that is hard to predict and painful to reproduce is how will it behave with old saves that saved mid-activity.

AngelicosPhosphoros · 2022-06-22T11:51:20Z

Alright, looks good on paper and I didn't manage to get it to break in manual testing.
It will probably bug out in some edge cases like all big changes inevitably do, so stay tuned for player feedback.
One thing that is hard to predict and painful to reproduce is how will it behave with old saves that saved mid-activity.

Well, I played with this patch 10 days and didn't got any issues yet.

Now I would begin Full stage of refactoring.

AngelicosPhosphoros mentioned this pull request Jun 11, 2022

Implement examine action for grid furniture #1612

Open

7 tasks

AngelicosPhosphoros force-pushed the make_grid_powered_appliances_usable branch from efd77b4 to d1136d2 Compare June 11, 2022 02:57

AngelicosPhosphoros changed the title ~~[CR] MVP of grid furniture examine action~~ [CR] [WIP] MVP of grid furniture examine action Jun 11, 2022

AngelicosPhosphoros force-pushed the make_grid_powered_appliances_usable branch from d1136d2 to 6de118d Compare June 11, 2022 11:19

Coolthulhu reviewed Jun 11, 2022

View reviewed changes

AngelicosPhosphoros force-pushed the make_grid_powered_appliances_usable branch 2 times, most recently from 4c8fd40 to f6ac3b9 Compare June 11, 2022 20:04

AngelicosPhosphoros changed the title ~~[CR] [WIP] MVP of grid furniture examine action~~ MVP of grid furniture examine action Jun 11, 2022

AngelicosPhosphoros requested a review from Coolthulhu June 14, 2022 01:58

Coolthulhu reviewed Jun 14, 2022

View reviewed changes

AngelicosPhosphoros force-pushed the make_grid_powered_appliances_usable branch from f6ac3b9 to 8ffaa86 Compare June 15, 2022 16:11

Coolthulhu self-assigned this Jun 19, 2022

Coolthulhu merged commit 3f8acfd into cataclysmbnteam:upload Jun 21, 2022

AngelicosPhosphoros deleted the make_grid_powered_appliances_usable branch June 22, 2022 11:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MVP of grid furniture examine action #1613

MVP of grid furniture examine action #1613

AngelicosPhosphoros commented Jun 11, 2022 •

edited

Loading

AngelicosPhosphoros commented Jun 11, 2022

Coolthulhu Jun 11, 2022

AngelicosPhosphoros Jun 11, 2022 •

edited

Loading

Coolthulhu Jun 11, 2022

AngelicosPhosphoros Jun 11, 2022

AngelicosPhosphoros Jun 12, 2022

AngelicosPhosphoros commented Jun 13, 2022

Coolthulhu Jun 14, 2022

AngelicosPhosphoros Jun 15, 2022

Coolthulhu Jun 14, 2022

AngelicosPhosphoros Jun 15, 2022

Coolthulhu Jun 14, 2022

AngelicosPhosphoros Jun 15, 2022

AngelicosPhosphoros commented Jun 15, 2022

Coolthulhu commented Jun 21, 2022

AngelicosPhosphoros commented Jun 22, 2022

MVP of grid furniture examine action #1613

MVP of grid furniture examine action #1613

Conversation

AngelicosPhosphoros commented Jun 11, 2022 • edited Loading

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

Additional context

AngelicosPhosphoros commented Jun 11, 2022

Choose a reason for hiding this comment

AngelicosPhosphoros Jun 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AngelicosPhosphoros commented Jun 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AngelicosPhosphoros commented Jun 15, 2022

Coolthulhu commented Jun 21, 2022

AngelicosPhosphoros commented Jun 22, 2022

AngelicosPhosphoros commented Jun 11, 2022 •

edited

Loading

AngelicosPhosphoros Jun 11, 2022 •

edited

Loading