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.

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

Move entity options #213

Closed
Buh13246 opened this issue Jan 3, 2020 · 5 comments
Closed

Move entity options #213

Buh13246 opened this issue Jan 3, 2020 · 5 comments

Comments

@Buh13246
Copy link

Buh13246 commented Jan 3, 2020

Is your feature request related to a problem? Please describe.
I have a friend who wants to have the options: movePassiveMobs and moveHostileMobs. Should I do a Pull request after I wrote these options or do you think these options shouldn't be included (seperate from origin)?

Second thing I think the onlyMovePlayers CraftType option should not default to true!
File: /modules/api/.../craft/CraftTypes.java Line: 163 (position based on commit 3dd6c7f)
onlyMovePlayers = (boolean) data.getOrDefault("onlyMovePlayers", true);

Reasons

  1. People could think that the moveEntity option doesn't work, because they didn't find the onlyMovePlayers option.
A Person: 12.12.2019
You should put in an issue about the moveEntities property not working.    
  1. Normally, a ship should be able to transport peacefull creatures per default.

Describe the solution you'd like
I will implement a movePassiveMobs(default: true) and moveHostileMobs(default: false) and change the default of onlyMovePlayer to false.

So every ship will be able to transport peaceful creatures, but zombies etc. won't be transported by default.

Is there a pull request interest after finishing?

Thx for reading.

@oh-noey
Copy link
Collaborator

oh-noey commented Jan 3, 2020

It might be a good idea to instead just allow a whitelist system for mobs that should be allowed to move instead, with the default as players, and than have the entry "hostile", "neutral", and "passive" as valid entries

@Buh13246
Copy link
Author

Buh13246 commented Jan 3, 2020

thats even better. thx. I will think about it

@Buh13246
Copy link
Author

Buh13246 commented Jan 3, 2020

I thought about representing the List with Interfaces: https://hub.spigotmc.org/javadocs/spigot/org/bukkit/entity/LivingEntity.html.

Benefits:

  1. we dont need to adjust things. if it's a Subinterface of LivingEntity, it's valid.
  2. it's completly configureable if i add the Interface AbstractHorse to my config, it automatically means: ChestedHorse, Donkey, Horse, Llama, Mule, SkeletonHorse, TraderLlama, ZombieHorse
  3. If there are Mods installed, these extra Mobs will be able to be configured without extra Code.

Problem:
How to Map a List of Interface names to Interfaces...

@Buh13246
Copy link
Author

Buh13246 commented Jan 4, 2020

Craft File

moveEntities: true
moveEntityList:
    - Animals
    - Zombie
    - Spider

Console Output

[09:13:34 INFO]: [Craft airskiff.craft] moveEntityList loaded Class org.bukkit.entity.Animals
[09:13:34 INFO]: [Craft airskiff.craft] moveEntityList loaded Class org.bukkit.entity.Zombie
[09:13:34 INFO]: [Craft airskiff.craft] moveEntityList loaded Class org.bukkit.entity.Spider
[09:13:34 INFO]: [Movecraft] Loaded 10 Craft files

I will start a Pull Request when the translation etc. works completely.

Buh13246 pushed a commit to Buh13246/Movecraft that referenced this issue Jan 4, 2020
…lse and added a moveEntityList option which can contain multiple Class/Interface Names from package org.bukkit.entity. Default is Player and Animals.
@TylerS1066 TylerS1066 changed the title new craft options Move entity options Jan 2, 2021
@TylerS1066 TylerS1066 moved this to New in Issue Triage Mar 23, 2022
@TylerS1066 TylerS1066 moved this from New to Backburner in Issue Triage Mar 23, 2022
@TylerS1066
Copy link
Contributor

This is now a duplicate of #398 which includes a more through description of the type of solution we would like to see.

Repository owner moved this from Backburner to Resolved in Issue Triage Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants