Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

port ore bag magnet toggle from white dream (DeltaV #2467) #2765

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

port ore bag magnet toggle from white dream (DeltaV #2467) #2765

wants to merge 18 commits into from

Conversation

dvir001
Copy link
Contributor

@dvir001 dvir001 commented Jan 22, 2025 •
edited by whatston3
Loading

About the PR

ore bags magnet is now a toggle instead of only if you equip it in your belt slot
this means mining borgs can now use ore magnet
DeltaV-Station/Delta-v#2467

Why / Balance

insane QOL for mining borgs and salvage in general

Media

Requirements

Breaking changes

N/A

Changelog
🆑 Hell_Cat, deltanedas

  • tweak: Bags with magnets (e.g. ore bag, plant bag, mail bag) can be toggled without being worn.

deltanedas and others added 3 commits January 22, 2025 23:37
@deltanedas @dvir001
port ore bag magnet toggle from white dream (new-frontiers-14#2467)
f2afa1e 
* add on textures for ore bags

* add code for toggling magnet and working in-hand

* update ore bag yml

---------

Co-authored-by: deltanedas <@deltanedas:kde.org>
@dvir001
Cleanup
db749bc
@dvir001
Merge branch 'master' into 2025-01-22-OreBag
ff90d05
@dvir001 dvir001 requested a review from whatston3 January 22, 2025 21:46
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

RSI Diff Bot; head commit 15296ea merging into 64d7b40
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Nyanotrasen/Clothing/Belt/mailbag.rsi

State Old New Status
icon-on Added
inhand-left Added
inhand-right Added

Resources/Textures/Objects/Specific/Chemistry/chem_bag.rsi

State Old New Status
icon-on Added

Resources/Textures/Objects/Specific/Hydroponics/Equipment/plant_bag.rsi

State Old New Status
icon-on Added

Resources/Textures/Objects/Specific/Library/Equipment/books_bag.rsi

State Old New Status
icon-on Added

Resources/Textures/Objects/Specific/Mining/ore_bag.rsi

State Old New Status
icon-on Added

Resources/Textures/_DV/Objects/Specific/Hydroponics/plant_bag_holding.rsi

State Old New Status
icon-on Added

Resources/Textures/_NF/Objects/Specific/Engineering/construction_bag.rsi

State Old New Status
icon-on-bluespace Added
icon-on Added

Resources/Textures/_NF/Objects/Specific/Mining/ore_bag_holding.rsi

State Old New Status
icon-on Added

Edit: diff updated after 15296ea

@github-actions github-actions bot added the S: Needs Review This PR is awaiting reviews label Jan 22, 2025
@whatston3
Copy link
Contributor

Seems like good stuff, would be good to have this on other bags (construction, plant, mail, book(?)) just for the visual feedback.

@github-actions github-actions bot added the S: Merge Conflict This PR has conflicts that prevent merging label Feb 4, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 4, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the S: Merge Conflict This PR has conflicts that prevent merging label Feb 4, 2025
@whatston3 whatston3 self-assigned this Feb 7, 2025
@whatston3
Copy link
Contributor

Works alright. Pushed a bunch of changes so all bags work the same way (books bag, chemistry bag, mail bag, plant bag, ore bag, and construction bag)
image

whatston3
whatston3 approved these changes Feb 7, 2025
View reviewed changes
Copy link
Contributor

@whatston3 whatston3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine, though it would be nice if the toggle verb was an alternate instead of a activation verb - being able to open the bag with Z/E and toggling the magnet with alt+Z/E would be a nice touch.

Good enough, though, and the bag consistency is alright, but verbose in YAML.

@whatston3 whatston3 added S: Approved and removed S: Needs Review This PR is awaiting reviews S: Untriaged labels Feb 7, 2025
@github-actions github-actions bot added the FTL label Feb 7, 2025
whatston3
whatston3 previously approved these changes Feb 7, 2025
View reviewed changes
Copy link
Contributor

@whatston3 whatston3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done nitpicking. It's good.

@whatston3 whatston3 dismissed their stale review February 8, 2025 22:08

making changes to get alt-use verbs

@whatston3 whatston3 added S: Awaiting Changes This PR has changes that need to be made before merging S: DO NOT MERGE and removed S: Approved labels Feb 8, 2025
@whatston3 whatston3 added S: Needs Review This PR is awaiting reviews and removed S: Awaiting Changes This PR has changes that need to be made before merging S: DO NOT MERGE labels Feb 9, 2025
@whatston3
Copy link
Contributor

whatston3 commented Feb 9, 2025 •
edited
Loading

Good news: alt-click works.
Bad news: now we effectively have two ways to enable magnets on things. 👍 (three including MaterialStorage magnets)

YAML configuration is currently a bit of a pig's ear, being both systems bodged together, but it does work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whatston3 whatston3 whatston3 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@whatston3 whatston3

Labels
C# FTL S: Needs Review This PR is awaiting reviews size/M Sprites YML
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dvir001 @whatston3 @deltanedas