Wuzzy/Repixture
6
33
Fork
You've already forked Repixture
10

player_api and crafting compatibility #190

Closed
mruncreative wants to merge 8 commits from mruncreative/Repixture:master into master
pull from: mruncreative/Repixture:master
merge into: Wuzzy:master
Wuzzy:master
Wuzzy:new_weather_types
Wuzzy:clock
Wuzzy:sponge
Wuzzy:cushion
Wuzzy:vase
Wuzzy:pages
Wuzzy:giant_crops
Wuzzy:duckweed
Wuzzy:new_hairs_new_skin_selection
Wuzzy:new_hairs
Wuzzy:multi_flower_2
Wuzzy:dirt_redo
Wuzzy:craft_compat
Wuzzy:gold_coin_upgrade
Wuzzy:gold_coin
Wuzzy:new_traders
Wuzzy:item_frame_flat
Wuzzy:silktouch_paint
Wuzzy:silktouch
Wuzzy:newtrees
Wuzzy:new_weather
Wuzzy:creative_per_player
Wuzzy:scrollbar_style
Wuzzy:village_nextgen_thread
Wuzzy:village_nextgen
Wuzzy:new_website
Wuzzy:tnt_faster
Wuzzy:fixbrush
Wuzzy:roots
Wuzzy:amber_slime
Wuzzy:fir
Wuzzy:path_fixes
Wuzzy:villager_pathfind_cost
Wuzzy:fence_gate
Wuzzy:slab_up
Wuzzy:biome_skies
Wuzzy:pencil_showlabel
Wuzzy:moonflower_seed
Wuzzy:unicode_signs_refactor_newsigns_sideways
Wuzzy:unicode_signs
Wuzzy:spyglass
Wuzzy:craftcolor
Wuzzy:villager_pathfind
Wuzzy:mobs_new2
Wuzzy:paint_book
Wuzzy:moon_phases
Wuzzy:newmusic
Wuzzy:weblate_test
Wuzzy:paint4dir
Wuzzy:whistle
Wuzzy:buoy
Wuzzy:pot
Wuzzy:4x4grass
Wuzzy:village_vmanip
Wuzzy:seagrass
First-time contributor
Copy link

renames rp_player to player_api, restores compatibility for most of its functions, updates the model files from MTG, adds mod crafting recipe compatibility code

Mods adding crafting recipes should add rp_crafting to optional dependencies.

renames rp_player to player_api, restores compatibility for most of its functions, updates the model files from MTG, adds mod crafting recipe compatibility code Mods adding crafting recipes should add rp_crafting to optional dependencies.
Owner
Copy link

No. I want rp_player to remain since it adds new feature. I also reserve the right to add more and incompatible features in future. I do not want Repixture to be restricted by Minetest Game.

My idea is to add player_api as a wrapper instead.

Rejected.

BUT!
I am willing to accept a separate PR with the player model fixed.
I am also willing to accept a separate PR with the crafting compability layer.

(Sorry, I am very strict with what I allow in Repixture.)

No. I want `rp_player` to remain since it adds new feature. I also reserve the right to add more and incompatible features in future. I do not want Repixture to be restricted by Minetest Game. My idea is to add `player_api` as a wrapper instead. Rejected. BUT! I am willing to accept a separate PR with the player model fixed. I am also willing to accept a separate PR with the crafting compability layer. (Sorry, I am very strict with what I allow in Repixture.)
Wuzzy closed this pull request 2024年06月09日 20:27:21 +02:00
Author
First-time contributor
Copy link

I don't think you understood. I didn't take away rp_player. The mod is simply renamed to player_api and compatibility is restored. Your extensions are still there.
Being artificially incompatible doesn't help anyone.

I don't think you understood. I didn't take away rp_player. The mod is simply renamed to player_api and compatibility is restored. Your extensions are still there. Being artificially incompatible doesn't help anyone.
mruncreative reopened this pull request 2024年06月09日 20:30:00 +02:00
Author
First-time contributor
Copy link

There is a single mod that is rp_player specific
https://github.com/sfence/adaptation_modpack/blob/master/adaptation_repixture/init.lua
Easier to fix it up than to insist upon being incompatible with literally everything else.

There is a single mod that is rp_player specific https://github.com/sfence/adaptation_modpack/blob/master/adaptation_repixture/init.lua Easier to fix it up than to insist upon being incompatible with literally everything else.
Author
First-time contributor
Copy link

Alright. I've named it back to rp_player. You can do compatibility stuff in the metatable function in player_api.lua if you change names of your functions and argument order to whatever you want.

Just noticed that "allow edits from maintainers" is turned off by default on this site. I've turned it on.

Alright. I've named it back to rp_player. You can do compatibility stuff in the metatable function in player_api.lua if you change names of your functions and argument order to whatever you want. Just noticed that "allow edits from maintainers" is turned off by default on this site. I've turned it on.
Author
First-time contributor
Copy link

Now all compatibility stuff is done in the wrapper. See #190/files

Now all compatibility stuff is done in the wrapper. See https://codeberg.org/Wuzzy/Repixture/pulls/190/files
Owner
Copy link

In the meantime, I just did it myself (currently untested).

https://codeberg.org/Wuzzy/Repixture/src/branch/master/mods/player_api

In the meantime, I just did it myself (currently untested). https://codeberg.org/Wuzzy/Repixture/src/branch/master/mods/player_api
Owner
Copy link

I would like it more if each PR only has one feature, and not multiple features mixed. Makes reviews harder.

I would like it more if each PR only has one feature, and not multiple features mixed. Makes reviews harder.
Owner
Copy link

I have looked at this again. Here are my comments:

  • I have already added a player_api compability layer (so it is no longer relevant in this PR)
  • The new player model is notably smaller and darker, so I can't accept it. Please make is so that the new player model is equal in size and has the same color (a tiny difference is OK, but only tiny)
  • The crafting compability probably does not work when a mod uses minetest.clear_craft. I think this function needs to be supported
I have looked at this again. Here are my comments: * I have already added a `player_api` compability layer (so it is no longer relevant in this PR) * The new player model is notably smaller and darker, so I can't accept it. Please make is so that the new player model is equal in size and has the same color (a tiny difference is OK, but only tiny) * The crafting compability probably does not work when a mod uses `minetest.clear_craft`. I think this function needs to be supported
Author
First-time contributor
Copy link

Alright. Just fix the sitting height on your model's animation and close the pull request. You can copy the sitting height adjustments for the boats from this PR.

Alright. Just fix the sitting height on your model's animation and close the pull request. You can copy the sitting height adjustments for the boats from this PR.
Wuzzy closed this pull request 2024年06月14日 01:23:20 +02:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
Labels
Clear labels
Category: Discussion
For discussions about the game
Category: Entities
About entities or objects (including player)
Category: Gameplay
About gameplay mechanics in general
Category: Graphics
About graphics
Category: GUI and HUD
Anything related to the graphical user interface or heads-up display
Category: Items and Tools
About items or tools, but not nodes
Category: Mapgen
About the map generator
Category: Nodes
About nodes
Category: Sounds
About sounds
Category: Translation
About translations
Category: Website
Related to the Repixture website
Kind
Balancing
 About game balance
Kind
Bug
 When something doesn't work right
Kind
Documentation
 About documentation, both for devs and players
Kind
Feature
 Ideas, feature requests, enhancements, etc.
Kind
Maintenance
 Code quality, technical improvements, refactoring, etc.
Kind
Performance
 About making the game run fast
Priority
CRITICAL
 Highest priority level. For crashes, major data loss, etc.
Priority
High
 The priority is high
Priority
Low
 The priority is low
Priority
Medium
 The priority is medium
Reviewed
Decision needed
 When the issue still needs a decision on what to do
Reviewed
Duplicate
 When the issue existed already
Reviewed
Invalid
 When the issue was not valid
Reviewed
Rejected
 When something that was proposed was rejected
Status
Needs engine change
 When it can only be solved after a Luanti bugfix/improvement
Status
Needs testing
 If the task is believed to be done, but needs testing
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Wuzzy/Repixture!190
Reference in a new issue
Wuzzy/Repixture
No description provided.
Delete branch "mruncreative/Repixture:master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?